Try to Implement my first Stub function - AbortPrinter().

[Loïc Maury]

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 ?

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;
     DWORD jobDocId;
     DWORD needed;
     LPWSTR portname;

#if defined(__APPLE__)

     EnterCriticalSection(&printer_handles_cs);

     printer = get_opened_printer(hPrinter);

     if(!printer)
     {
         ERR("The handle for the printer is invalid.\n");
         SetLastError(ERROR_INVALID_HANDLE);
         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");
         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;
                 ret = TRUE;

                 if(pi5)
                     HeapFree(GetProcessHeap(), 0, pi5);
             }
             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
}

Merci

Loïc

[Nikolay Sivov]

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

[Loïc Maury]

Hello Mr.Sivov and Wine community,

On 03/02/11 12:02, Nikolay Sivov wrote:
> 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.
> > OK I understand.
> > 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.
> > Ok
> >     DWORD jobDocId;
> >     DWORD needed;
> >     LPWSTR portname;
> >
> > #if defined(__APPLE__)
> > Why it's Mac specific?
> > In fact I don't know if I need this, but I am on Mac with CUPS, I don't 
> > know if it the same
> > issue for the other platform ?
> >
> > 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.
> > Ok
> >         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.
> > Yes, I can remove this.
> > 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?
> Yes, sorry it's not necessary.
> > ret = TRUE;
> >
> > if(pi5)
> >                     HeapFree(GetProcessHeap(), 0, pi5);
>
> No need to check point here.
> Ok I will move it after the "end:" label.
> > }
> >             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.
> Ok I will change it to FIXME.

Thank you

Loïc
> > }
> >
> > Merci
> >
> > Loïc

[Charles Davis]

On 2/4/11 6:53 AM, Loïc Maury wrote:
> On 03/02/11 12:02, Nikolay Sivov wrote:
> > Why it's Mac specific?
> > In fact I don't know if I need this, but I am on Mac with CUPS, I don't
> > know if it the same
> > issue for the other platform ?
> > Probably. Every UNIX I've seen uses CUPS (short for Common UNIX Printing
> > System). There's also HPLIP (Hewlett-Packard Linux Imaging and Printing
> > System), but I've never seen any programs that use that directly, and
> > I'm not even sure if it still exists anymore :).

Chip

[Henri Verbeet]

On 4 February 2011 15:30, Charles Davis <cdavis(a)mymail.mines.edu> wrote:
> Probably. Every UNIX I've seen uses CUPS (short for Common UNIX Printing
> CUPS is probably pretty common these days, but LPD has been around for
> a long time. If it's really CUPS specific it should check for CUPS
> though, not OS X. Also note that the code now has unused variables in
> the #else case.