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

[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
>>
>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20110204/551c2c73/attachment.htm>