Re: Try to Implement my first Stub function - AbortPrinter() - (try 2).

[Loïc Maury]

Hello Mr.Le Cam

On Tue, Feb 8, 2011 at 10:07 PM, Nicolas Le Cam wrote:
> 2011/2/8 Loïc Maury
> > Hello Mr.Timoshkov,
> >
> > Thank you for your reply.
> > > Loīc Maury  wrote:
> > >
> > > After the various comments, I have modified the patch.
> > > >
> > >
> > > First of all set your tab size to 8, and ask your editor to not use tabs
> > > at all.
> >
> > I have modified my editor, but I don't know if it 's correct now ?
> > > -
> > >   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));
> > >
> > > TRACE() with the API parameters usually is the very first statement in
> > > the API implementation, comma should be placed at the end of the
> > > statement,
> > > not before.
> >
> > Ok, I have remplaced this TRACE().
> > > -
> > >                           GetPrinterW(hPrinter, 2, NULL,
> > > > 0,&needed);
> > > > -
> > > >                           pi2 = HeapAlloc(GetProcessHeap(), 0,
> > > >
> > > > needed);
> > > > -
> > > >                           GetPrinterW(hPrinter, 2, (LPBYTE)pi2,
> > > >
> > > > needed,&needed);
> > >
> > > You need to check the return value of GetPrinterW() and handle the
> > > errors.
> >
> > Ok
> > > -
> > >   if(pi2)
> > > > -
> > > >           HeapFree(GetProcessHeap(), 0, pi2);
> > >
> > > NULL check before HeapFree() is not needed.
> >
> > Ok, I have removed the NULL check.
> > - TRACE("return %d\n", ret);
> > > >
> > >
> > > This trace is redundant.
> >
> > Ok, I have removed this TRACE().
> >
> > I have make an other patch.
> >
> > Thank you
> >
> > Loīc
>
> Hi Loïc,
>
> In addition to Dmitry and Andrew comments, you're adding a lot of trailing
> spaces (git apply output : warning: squelched 22 whitespace errors warning:
> 27 lines add whitespace errors.), there's also no need for an extra newline
> after the end label.

Now, The patch is correct ?

Thank you

Loïc
> --
> Nicolas Le Cam