[PATCH 1/9] hhctrl.ocx: Implement HH_CLOSE_ALL (resend).

Jacek Caban jacek at codeweavers.com
Thu Jul 26 09:46:26 CDT 2012


On 07/10/12 19:12, Erich E. Hoover wrote:
> On Tue, Jul 10, 2012 at 3:52 AM, Jacek Caban <jacek at 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



More information about the wine-devel mailing list