[PATCH] user32: Don't depend on HMENU being valid for popup menus
Andrew Eikum
aeikum at codeweavers.com
Wed May 7 08:55:09 CDT 2014
On Tue, May 06, 2014 at 04:34:21PM -0500, Vincent Povirk wrote:
> if (menu->FocusedItem != NO_SELECTED_ITEM)
> {
> + ERR("focused: %u, nitms: 0x%x\n",
> + menu->FocusedItem,
> + menu->nItems);
> menu->items[menu->FocusedItem].fState &= ~(MF_HILITE|MF_MOUSESELECT);
> menu->FocusedItem = NO_SELECTED_ITEM;
> }
>
> Is this stray debugging code or a real error condition?
>
Yes, it's debugging code. The dangers of sending really large
patches...
> I'm not sure this frees submenus correctly. If I create a popup menu
> and add a submenu to it, that submenu has two references (one because
> it has an HMENU and one because it is a submenu). DestroyMenu should
> recursively destroy submenus. So it seems to me, destroying a menu
> needs to destroy the handles to submenus (if they have one) and
> release its references to them.
>
We don't have any tests for this, though MSDN agrees with you. I'm
surprised that adding a submenu transfers the HMENU ownership to the
owning menu. I'll add a DestroyMenu call for submenus in
MENU_ReleaseMenu, which should close the handle and release the final
reference.
I fixed the other errors you pointed out. Thanks for reviewing!
Andrew
More information about the wine-devel
mailing list