On 2/3/2011 13:51, Loïc Maury wrote:

Hello,

I try to implement my first stub function - AbortPrinter().
But before to make a patch, I need your advice.

For what I understood, AbortPrinter(), remove the document
spool file for a printer, created by StartDocPrinter(), who indicate
that a document was spooled.

I saw that the API printer functions are in dlls/winspool.drv/info.c

I understood that there are 4 structures used, opened_printer_t for manage a printer,
jobqueue_t for a list of job for a printer, started_doc_t who manage a document,
and job_t for manage a job.

I am on Mac Os 10.6, with CUPS.

Here is the code, what do you think ?

Patch needs to be a diff, not a copy of a call. Some useful links http://wiki.winehq.org/GitWine, http://wiki.winehq.org/SubmittingPatches.

BOOL WINAPI AbortPrinter(HANDLE hPrinter)
{
    PRINTER_INFO_5W *pi5 = NULL;
    BOOL ret = FALSE;
    opened_printer_t *printer;
    struct list *cursor, *cursor2;
    job_t *job = 0;

No need to init 'job' it seems.

    DWORD jobDocId;
    DWORD needed;
    LPWSTR portname;
   
#if defined(__APPLE__)

Why it's Mac specific?

    EnterCriticalSection(&printer_handles_cs);
   
    printer = get_opened_printer(hPrinter);
   
    if(!printer)
    {
        ERR("The handle for the printer is invalid.\n");
        SetLastError(ERROR_INVALID_HANDLE);

If you set last error you should add a test for it.

        goto end;
    }
   
    TRACE("(%s, %s, %p, %d, %d)\n",debugstr_w(printer->name)
                                  ,debugstr_w(printer->printername)
                                  ,printer->backend_printer
                                  ,printer->queue->ref
                                  ,list_count(&printer->queue->jobs));
   
    /* No jobs to manage. */
    if(list_count(&printer->queue->jobs) == 0)
    {
        ERR("No job in the list.\n");

It's not an error to have no jobs.

        SetLastError(ERROR_SPL_NO_STARTDOC);
        goto end;
    }
   
    if(printer->doc) {
        TRACE("Document inside for job id : %d\n", printer->doc->job_id);
        jobDocId = printer->doc->job_id;
    }
    else {
        ERR("No document.\n");
        SetLastError(ERROR_SPL_NO_STARTDOC);
        goto end;
    }
   
    /* For each jobs, get the job document in the double linked list */
    LIST_FOR_EACH_SAFE(cursor, cursor2, &printer->queue->jobs)
    {
        /* Take a job. */
        job = LIST_ENTRY(cursor, job_t, entry);
       
        TRACE("(job id : %d, filename : %s, portname : %s, document title : %s, printer name %s)\n",job->job_id
                                                                                                   ,debugstr_w(job->filename)
                                                                                                   ,debugstr_w(job->portname)
                                                                                                   ,debugstr_w(job->document_title)
                                                                                                   ,debugstr_w(job->printer_name));
        if(jobDocId == job->job_id)
        {   
            TRACE("(hf : %p, job id : %d)\n",printer->doc->hf
                                            ,printer->doc->job_id);
           
            /* Get portname. */
            if (!job->portname)
            {
                GetPrinterW(hPrinter, 5, NULL, 0, &needed);
                pi5 = HeapAlloc(GetProcessHeap(), 0, needed);
                GetPrinterW(hPrinter, 5, (LPBYTE)pi5, needed, &needed);
                portname = pi5->pPortName;
            }
           
            TRACE("portname : %s\n", debugstr_w(portname));

            /* Cups Port */
            if(!strncmpW(portname, CUPS_Port, strlenW(CUPS_Port)))
            {
                TRACE("Remove job from the list.\n");
                list_remove(cursor);
                CloseHandle(printer->doc->hf);
                DeleteFileW(job->filename);
                HeapFree(GetProcessHeap(), 0, job->document_title);
                HeapFree(GetProcessHeap(), 0, job->printer_name);
                HeapFree(GetProcessHeap(), 0, job->portname);
                HeapFree(GetProcessHeap(), 0, job->filename);
                HeapFree(GetProcessHeap(), 0, job->devmode);
                HeapFree(GetProcessHeap(), 0, job);

                job = 0;

What is it for?

                ret = TRUE;
               
                if(pi5)
                    HeapFree(GetProcessHeap(), 0, pi5);

No need to check point here.

            }
            else
                FIXME("only CUPS for now.\n");
        }
    }
   
end:   
    LeaveCriticalSection(&printer_handles_cs);
   
    TRACE("return %d\n", ret);
    return ret;

#else
    TRACE("Only CUPS for Mac Os is managed for now.\n");
    return FALSE;
#endif   

This should stay as FIXME obviously.

}

Merci

Loïc