Try to implement my first stub function - AbortPrinter() - (try 3)

[Andrew Eikum]

Hi Loïc,

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, 
printer->doc->job_id);
+            TRACE("portname : %s\n", debugstr_w(portname));
+                TRACE("Remove job from the list.\n");
+                TRACE("Remove document for the printer : %s.\n", 
debugstr_w(printer->printername));

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);
+                if(!retFromGetPrinter)
+                    goto end;

You can just do "if(!GetPrinterW(...))" and remove the local variable.

Thanks for contributing,
Andrew