On Thu, 02 Aug 2007 16:06:44 +0200, Juan Lang juan.lang@gmail.com wrote:
Thank you. Hope it will be better this time. Also, I'll send a test if this is OK.
This one looks pretty good to me, but I have one error and a few nits to correct:
The error:
- {
- ERR("GetLocaleInfo of 0x%x failed in 2nd stage?!\n", localeValue);
- SysFreeString(*pbstrOut);
You should set *pbstrOut to NULL in this case.
OK. However, note that this was copy&pasted, so there are more instances of this error in wine. It could be somehow automated (like having a macro for these disposing functions which will also set the pointer to NULL).
The nits: You have a couple small errors in the comments:
- You misspell iWeekday
- The comment for iWeekday in the params section implies 0 is allowed,
but the first check in the function disallows it.
This is the problem the misspelled comment is about, MSDN also suggest 0 for weekday is OK, however, disassemble showed it isn't on Win XP.
Attaching a test with the patch would certainly help.
I'm sending it to wine-patches.
Regards Jiri Palecek