Try to implement my first stub function - AbortPrinter() - (try 3)
aeikum at codeweavers.com
Fri Apr 8 08:21:21 CDT 2011
On 04/08/2011 07:54 AM, Loïc Maury wrote:
> I try always to implement my first stub function - AbortPrinter.
It's looking good. Before you submit this, you should add some tests.
Take a look in <dlls/winspool.drv/tests/>. Demonstrate that
AbortPrinter() on Windows behaves the same way as your function does.
Make sure to test the value of GetLastError() on failure. You can run
your tests through the testbot <https://testbot.winehq.org/> before
submitting. Look at other tests for some guidance.
Below are a couple of tiny suggestions.
+ ERR("The handle for the printer is invalid.\n");
+ TRACE("Document inside for job id : %d\n", printer->doc->job_id);
+ ERR("No document.\n");
+ TRACE("(hf : %p, job id : %d)\n", printer->doc->hf,
+ TRACE("portname : %s\n", debugstr_w(portname));
+ TRACE("Remove job from the list.\n");
+ TRACE("Remove document for the printer : %s.\n",
You're outputting a lot of debug information. The ERRs aren't really
errors, but just invalid values from the application which are handled
appropriately. No problem here, so no need to ERR.
The other TRACE statements might be useful for debugging this particular
function, but can clutter the output and make future maintenance more
difficult. I'd remove them.
+ /* For each jobs, see if we have a job document in the double
linked list */
+ /* Get portname. */
+ /* Cups Port */
+ /* The document for the printer is not useful anymore. */
These comments are entirely redundant. The code is perfectly clear, and
the comments will just rot if this code is updated later. Don't bother
with comments like these unless you're doing something very unclear.
+ retFromGetPrinter = GetPrinterW(hPrinter, 2,
(LPBYTE)pi2, needed, &needed);
+ goto end;
You can just do "if(!GetPrinterW(...))" and remove the local variable.
Thanks for contributing,
More information about the wine-devel