[advpack/setupapi] LaunchINFSection

[Raphael]

Hi,

On Thursday 17 November 2005 04:45, Robert Shearman wrote:
> I have a few comments:

You too :)

>
> Why bother with the assert? You'll crash more nicely by just
> dereferencing the NULL pointer. At least then you'll get a backtrace in
> the debugger.

because i used to use assert in many of my codes 
(and i think that winedbg catch assertions). 

>
> This is just personal preference, but the extra brackets there aren't
> needed and don't add to the readability and I haven't seen this style
> used elsewhere in the Wine source code.

I used it but you are right i forget it in this code 

> Is this left over debugging code?

I prefer to speak about currently unsupported pathes

> >+      if (!bTest) return E_FAIL;
>
> Don't use E_FAIL. Try to return a proper error code, preferably using
> what native returns in this circumstance by adding a test case.

see 
http://msdn.microsoft.com/workshop/delivery/download/overview/launchinfsectionex.asp
Native function returns E_FAIL

>
> Can you give us a clue as to what the difference between
> LaunchINFSection and LaunchINFSectionEx might be by adding a comment? If
> you don't know, a comment would still be appreciated saying that it
> should be doing something different here.

you are right

> >+ * @param CabName: Cabinet file path
> >+ * @param ExpandDir: Extraction Output Directory
> >+ * @param Flags: UNKNOWN
> >+ * @param FileList: if NULL extract all files
> >+ *                  else UNKNOWN
> >+ *
>
> I don't think that is a c2man compatible documentation format.

No this should be fixed


Regards,
Raphael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://www.winehq.org/pipermail/wine-devel/attachments/20051121/b947c037/attachment.pgp