On 07/10/12 19:12, Erich E. Hoover wrote:
On Tue, Jul 10, 2012 at 3:52 AM, Jacek Caban jacek@codeweavers.com wrote:
... From what I see in patch 5, you also call list_add_tail outside constructor. ...
I don't see much of a choice, you need to be able to "set" wintype data for windows that don't exist yet. I've attached a proposed update including the other suggestions you've made, let me know what you think.
It still may be moved to common place, perhaps by splitting allocation and initialization to separated functions, but I'm fine with this solution for now. I have just minor comments:
info = CreateHelpViewer(fullname); if(!info) + { + heap_free((WCHAR*)default_index); + heap_free((WCHAR*)window); return NULL; + }
This is already existing problem I noticed while reviewing the patch that those calls should not be needed (resolve_filename should not return const strings). It's worth fixing before spreading those casts in heap_free calls (they are present in other patch as well).
+ HHInfo *info, *next; + + LIST_FOR_EACH_ENTRY_SAFE(info, next, &window_list, HHInfo, entry) + { + TRACE("Destroying window %s.\n", debugstr_w(info->WinType.pszType)); + if (info) + ReleaseHelpViewer(info);
if(info) is not needed here.
Thanks, Jacek