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
pgpjAvTHC8wtP.pgp
Description: PGP signature
