Re: PATCH: ParseX509EncodedCertificateForListBoxEntry stub

[Henri Verbeet]

I suppose I'll give Austin a hand because I like the occasional Valgrind runs.

On 11 June 2015 at 11:24, Marcus Meissner  wrote:
> +DWORD WINAPI ParseX509EncodedCertificateForListBoxEntry(LPBYTE cert, DWORD len, LPSTR szlistbox, LPDWORD listbox)

Please avoid the pointer typedefs and Hungarian. I.e., "char
*listbox", "DWORD *listbox_size", etc.
> - FIXME("stub: %p %d %s %p\n", cert, len, debugstr_a(szlistbox), listbox);

I'm not a particular fan of this tracing style, but I suppose it's the
local wininet style. "len" is a DWORD, so unsigned, so %u. "szlistbox"
is an output parameter, so tracing it as a string makes no sense.
> -@ stub ParseX509EncodedCertificateForListBoxEntry
> +@ stdcall ParseX509EncodedCertificateForListBoxEntry(ptr long str ptr)

Similarly, "str" is wrong for output parameters.
> +DWORD       WINAPI ParseX509EncodedCertificateForListBoxEntry(LPBYTE,DWORD,LPSTR,LPDWORD);

While not strictly wrong, I really think it's much more readable to
include the parameter names in the prototypes. I suppose that's local
wininet style too, but IMHO we should just fix it in that regard.
Also, pointer typedefs again.

PATCH: ParseX509EncodedCertificateForListBoxEntry stub

[Marcus Meissner]

On Thu, Jun 11, 2015 at 01:32:14PM +0200, Henri Verbeet wrote:
> I suppose I'll give Austin a hand because I like the occasional Valgrind runs.
>
> On 11 June 2015 at 11:24, Marcus Meissner  wrote:
> > +DWORD WINAPI ParseX509EncodedCertificateForListBoxEntry(LPBYTE cert, DWORD len, LPSTR szlistbox, LPDWORD listbox)
>
> Please avoid the pointer typedefs and Hungarian. I.e., "char
> *listbox", "DWORD *listbox_size", etc.

Fixed.
> > - FIXME("stub: %p %d %s %p\n", cert, len, debugstr_a(szlistbox), listbox);
>
> I'm not a particular fan of this tracing style, but I suppose it's the
> local wininet style. "len" is a DWORD, so unsigned, so %u. "szlistbox"
> is an output parameter, so tracing it as a string makes no sense.

I noticed that this was the main problem, yes.
> > -@ stub ParseX509EncodedCertificateForListBoxEntry
> > +@ stdcall ParseX509EncodedCertificateForListBoxEntry(ptr long str ptr)
>
> Similarly, "str" is wrong for output parameters.
> > +DWORD       WINAPI ParseX509EncodedCertificateForListBoxEntry(LPBYTE,DWORD,LPSTR,LPDWORD);
>
> While not strictly wrong, I really think it's much more readable to
> include the parameter names in the prototypes. I suppose that's local
> wininet style too, but IMHO we should just fix it in that regard.
> Also, pointer typedefs again.

The rest of the include file does not add parameter names there either, so I leave that.

Ciao, Marcus