[PATCH 6/6] user32: Don't rely on HMENU in internal menu structs

Nikolay Sivov nsivov at codeweavers.com
Tue Apr 10 01:36:55 CDT 2018


From: Andrew Eikum <aeikum at codeweavers.com>

Signed-off-by: Nikolay Sivov <nsivov at codeweavers.com>
---
 dlls/user32/menu.c       | 141 +++++++++++++++++++++++++----------------------
 dlls/user32/tests/menu.c |   4 +-
 2 files changed, 77 insertions(+), 68 deletions(-)

diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c
index c8d30eba17..284e48fab5 100644
--- a/dlls/user32/menu.c
+++ b/dlls/user32/menu.c
@@ -62,13 +62,15 @@
 WINE_DEFAULT_DEBUG_CHANNEL(menu);
 WINE_DECLARE_DEBUG_CHANNEL(accel);
 
+typedef struct _POPUPMENU POPUPMENU;
+
 /* Menu item structure */
 typedef struct {
     /* ----------- MENUITEMINFO Stuff ----------- */
     UINT fType;			/* Item type. */
     UINT fState;		/* Item state.  */
     UINT_PTR wID;		/* Item id.  */
-    HMENU hSubMenu;		/* Pop-up menu.  */
+    POPUPMENU *submenu;         /* Pop-up menu.  */
     HBITMAP hCheckBit;		/* Bitmap when checked.  */
     HBITMAP hUnCheckBit;	/* Bitmap when unchecked.  */
     LPWSTR text;		/* Item text. */
@@ -84,7 +86,8 @@ typedef struct {
 } MENUITEM;
 
 /* Popup menu structure */
-typedef struct {
+struct _POPUPMENU
+{
     struct user_object obj;
     WORD        wFlags;       /* Menu flags (MF_POPUP, MF_SYSMENU) */
     WORD	Width;        /* Width of the whole menu */
@@ -107,7 +110,7 @@ typedef struct {
     DWORD	dwMenuData;	/* application defined value */
     HMENU       hSysMenuOwner;  /* Handle to the dummy sys menu holder */
     WORD        textOffset;     /* Offset of text when items have both bitmaps and text */
-} POPUPMENU, *LPPOPUPMENU;
+};
 
 /* internal flags for menu tracking */
 
@@ -229,8 +232,8 @@ static void do_debug_print_menuitem(const char *prefix, const MENUITEM *mp,
     if (mp) {
         UINT flags = mp->fType;
         TRACE( "{ ID=0x%lx", mp->wID);
-        if ( mp->hSubMenu)
-            TRACE( ", Sub=%p", mp->hSubMenu);
+        if ( mp->submenu)
+            TRACE( ", Sub=%p", mp->submenu);
         if (flags) {
             int count = 0;
             TRACE( ", fType=");
@@ -570,7 +573,7 @@ static MENUITEM *MENU_FindItem( POPUPMENU **pmenu, UINT *nPos, UINT wFlags )
 	{
 	    if (item->fType & MF_POPUP)
 	    {
-                POPUPMENU *submenu = MENU_GetMenu(item->hSubMenu);
+                POPUPMENU *submenu = item->submenu;
                 MENUITEM *subitem = MENU_FindItem( &submenu, nPos, wFlags );
 		if (subitem)
 		{
@@ -617,12 +620,12 @@ static UINT MENU_FindSubMenu( POPUPMENU **pmenu, POPUPMENU *subTarget )
     item = menu->items;
     for (i = 0; i < menu->nItems; i++, item++) {
         if(!(item->fType & MF_POPUP)) continue;
-        if (item->hSubMenu == MENU_GetHandle(subTarget))
+        if (item->submenu == subTarget)
         {
             return i;
         }
         else  {
-            POPUPMENU *submenu = MENU_GetMenu(item->hSubMenu);
+            POPUPMENU *submenu = item->submenu;
             UINT pos = MENU_FindSubMenu( &submenu, subTarget );
             if (pos != NO_SELECTED_ITEM) {
                 *pmenu = submenu;
@@ -1131,7 +1134,7 @@ static void MENU_CalcItemSize( HDC hdc, MENUITEM *lpitem, HWND hwndOwner,
  *
  * Calculate the size of a popup menu.
  */
-static void MENU_PopupMenuCalcSize( LPPOPUPMENU lppop, UINT max_height )
+static void MENU_PopupMenuCalcSize( POPUPMENU *lppop, UINT max_height )
 {
     MENUITEM *lpitem;
     HDC hdc;
@@ -1237,7 +1240,7 @@ static void MENU_PopupMenuCalcSize( LPPOPUPMENU lppop, UINT max_height )
  * Calculate the size of the menu bar.
  */
 static void MENU_MenuBarCalcSize( HDC hdc, LPRECT lprect,
-                                  LPPOPUPMENU lppop, HWND hwndOwner )
+                                  POPUPMENU *lppop, HWND hwndOwner )
 {
     MENUITEM *lpitem;
     UINT start, i, helpPos;
@@ -1807,7 +1810,7 @@ static void MENU_DrawPopupMenu( HWND hwnd, HDC hdc, HMENU hmenu )
  */
 UINT MENU_DrawMenuBar( HDC hDC, LPRECT lprect, HWND hwnd )
 {
-    LPPOPUPMENU lppop;
+    POPUPMENU *lppop;
     HMENU hMenu = GetMenu(hwnd);
 
     lppop = MENU_GetMenu( hMenu );
@@ -1943,7 +1946,7 @@ static BOOL MENU_ShowPopup( HWND hwndOwner, POPUPMENU *menu, UINT id, UINT flags
  *           MENU_EnsureMenuItemVisible
  */
 static void
-MENU_EnsureMenuItemVisible(LPPOPUPMENU lppop, UINT wIndex, HDC hdc)
+MENU_EnsureMenuItemVisible(POPUPMENU *lppop, UINT wIndex, HDC hdc)
 {
     if (lppop->bScrolling)
     {
@@ -2254,7 +2257,7 @@ static LPCSTR MENUEX_ParseResource( LPCSTR res, HMENU hMenu)
 /***********************************************************************
  *           MENU_GetSubPopup
  *
- * Return the handle of the selected sub-popup menu (if any).
+ * Return the selected sub-popup menu (if any).
  */
 static POPUPMENU *MENU_GetSubPopup( POPUPMENU *menu )
 {
@@ -2264,7 +2267,7 @@ static POPUPMENU *MENU_GetSubPopup( POPUPMENU *menu )
 
     item = &menu->items[menu->FocusedItem];
     if ((item->fType & MF_POPUP) && (item->fState & MF_MOUSESELECT))
-        return MENU_GetMenu(item->hSubMenu);
+        return item->submenu;
 
     return NULL;
 }
@@ -2291,7 +2294,7 @@ static void MENU_HideSubPopups( HWND hwndOwner, POPUPMENU *menu,
 	    if (!(item->fType & MF_POPUP) ||
 		!(item->fState & MF_MOUSESELECT)) return;
 	    item->fState &= ~MF_MOUSESELECT;
-	    submenu = MENU_GetMenu(item->hSubMenu);
+	    submenu = item->submenu;
 	} else return;
 
         if (!submenu) return;
@@ -2336,7 +2339,7 @@ static POPUPMENU *MENU_ShowSubPopup( HWND hwndOwner, POPUPMENU *menu,
 
     /* Send WM_INITMENUPOPUP message only if TPM_NONOTIFY flag is not specified */
     if (!(wFlags & TPM_NONOTIFY))
-       SendMessageW( hwndOwner, WM_INITMENUPOPUP, (WPARAM)item->hSubMenu,
+       SendMessageW( hwndOwner, WM_INITMENUPOPUP, (WPARAM)MENU_GetHandle(item->submenu),
                      MAKELPARAM( menu->FocusedItem, IS_SYSTEM_MENU(menu) ));
 
     item = &menu->items[menu->FocusedItem];
@@ -2361,7 +2364,7 @@ static POPUPMENU *MENU_ShowSubPopup( HWND hwndOwner, POPUPMENU *menu,
 
     if (IS_SYSTEM_MENU(menu))
     {
-	MENU_InitSysMenuPopup(item->hSubMenu,
+        MENU_InitSysMenuPopup(MENU_GetHandle(item->submenu),
                               GetWindowLongW( menu->hWnd, GWL_STYLE ),
                               GetClassLongW( menu->hWnd, GCL_STYLE));
 
@@ -2405,13 +2408,13 @@ static POPUPMENU *MENU_ShowSubPopup( HWND hwndOwner, POPUPMENU *menu,
     /* use default alignment for submenus */
     wFlags &= ~(TPM_CENTERALIGN | TPM_RIGHTALIGN | TPM_VCENTERALIGN | TPM_BOTTOMALIGN);
 
-    MENU_InitPopup( hwndOwner, MENU_GetMenu(item->hSubMenu), wFlags );
+    MENU_InitPopup( hwndOwner, item->submenu, wFlags );
 
-    MENU_ShowPopup( hwndOwner, MENU_GetMenu(item->hSubMenu), menu->FocusedItem, wFlags,
+    MENU_ShowPopup( hwndOwner, item->submenu, menu->FocusedItem, wFlags,
 		    rect.left, rect.top, rect.right, rect.bottom );
     if (selectFirst)
-        MENU_MoveSelection( hwndOwner, MENU_GetMenu(item->hSubMenu), ITEM_NEXT );
-    return MENU_GetMenu(item->hSubMenu);
+        MENU_MoveSelection( hwndOwner, item->submenu, ITEM_NEXT );
+    return item->submenu;
 }
 
 
@@ -2452,7 +2455,7 @@ static POPUPMENU *MENU_PtMenu( POPUPMENU *menu, POINT pt )
    ret = (item != NO_SELECTED_ITEM &&
           (menu->items[item].fType & MF_POPUP) &&
           (menu->items[item].fState & MF_MOUSESELECT))
-        ? MENU_PtMenu(MENU_GetMenu(menu->items[item].hSubMenu), pt) : 0;
+        ? MENU_PtMenu(menu->items[item].submenu, pt) : 0;
 
    if (!ret)  /* check the current window (avoiding WM_HITTEST) */
    {
@@ -2490,7 +2493,7 @@ static INT MENU_ExecFocusedItem( MTRACKER* pmt, POPUPMENU *menu, UINT wFlags )
 
     item = &menu->items[menu->FocusedItem];
 
-    TRACE("menu %p wID %08lx hSubMenu %p fType %04x\n", menu, item->wID, item->hSubMenu, item->fType);
+    TRACE("menu %p wID %08lx hSubMenu %p fType %04x\n", menu, item->wID, item->submenu, item->fType);
 
     if (!(item->fType & MF_POPUP))
     {
@@ -3568,7 +3571,7 @@ UINT MENU_GetMenuBarHeight( HWND hwnd, UINT menubarWidth,
 {
     HDC hdc;
     RECT rectBar;
-    LPPOPUPMENU lppop;
+    POPUPMENU *lppop;
 
     TRACE("HWND %p, width %d, at (%d, %d).\n", hwnd, menubarWidth, orgX, orgY );
 
@@ -3771,7 +3774,7 @@ UINT WINAPI GetMenuState( HMENU hMenu, UINT wItemID, UINT wFlags )
     debug_print_menuitem ("  item: ", item, "");
     if (item->fType & MF_POPUP)
     {
-        menu = MENU_GetMenu( item->hSubMenu );
+        menu = item->submenu;
 	if (!menu) return -1;
 	else return (menu->nItems << 8) | ((item->fState|item->fType) & 0xff);
     }
@@ -3790,7 +3793,7 @@ UINT WINAPI GetMenuState( HMENU hMenu, UINT wItemID, UINT wFlags )
  */
 INT WINAPI GetMenuItemCount( HMENU hMenu )
 {
-    LPPOPUPMENU	menu = MENU_GetMenu(hMenu);
+    POPUPMENU *menu = MENU_GetMenu(hMenu);
     if (!menu) return -1;
     TRACE("(%p) returning %d\n", hMenu, menu->nItems );
     return menu->nItems;
@@ -3940,7 +3943,7 @@ BOOL WINAPI RemoveMenu( HMENU hMenu, UINT nPos, UINT wFlags )
 
     /* Remove item */
     if (item->fType & MF_POPUP)
-        MENU_ReleaseMenu(MENU_GetMenu(item->hSubMenu));
+        MENU_ReleaseMenu(item->submenu);
 
     MENU_FreeItemData( item );
 
@@ -3974,9 +3977,9 @@ BOOL WINAPI DeleteMenu( HMENU hMenu, UINT nPos, UINT wFlags )
     POPUPMENU *menu = MENU_GetMenu(hMenu);
     MENUITEM *item = MENU_FindItem( &menu, &nPos, wFlags );
     if (!item) return FALSE;
-    if (item->fType & MF_POPUP) DestroyMenu( item->hSubMenu );
+    if (item->fType & MF_POPUP) DestroyMenu( MENU_GetHandle(item->submenu) );
       /* nPos is now the position of the item */
-    RemoveMenu( hMenu, nPos, wFlags | MF_BYPOSITION );
+    RemoveMenu( MENU_GetHandle(menu), nPos, wFlags | MF_BYPOSITION );
     return TRUE;
 }
 
@@ -4088,7 +4091,7 @@ BOOL WINAPI SetMenuItemBitmaps( HMENU hMenu, UINT nPos, UINT wFlags,
 HMENU WINAPI CreateMenu(void)
 {
     HMENU hMenu;
-    LPPOPUPMENU menu;
+    POPUPMENU *menu;
 
     if (!(menu = HeapAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*menu) ))) return 0;
     menu->FocusedItem = NO_SELECTED_ITEM;
@@ -4128,8 +4131,8 @@ static BOOL MENU_ReleaseMenu( POPUPMENU *lppop )
             if (item->fType & MF_POPUP)
             {
                 /* Release handle if it hasn't already been released. */
-                if (!MENU_ReleaseMenu(MENU_GetMenu(item->hSubMenu)))
-                    DestroyMenu(item->hSubMenu);
+                if (!MENU_ReleaseMenu(item->submenu))
+                    DestroyMenu(MENU_GetHandle(item->submenu));
             }
             MENU_FreeItemData( item );
         }
@@ -4300,7 +4303,7 @@ BOOL WINAPI GetMenuBarInfo( HWND hwnd, LONG idObject, LONG idItem, PMENUBARINFO
         pmbi->fFocused = menu->FocusedItem == idItem - 1;
         if (pmbi->fFocused && (menu->items[idItem - 1].fType & MF_POPUP))
         {
-            menu = MENU_GetMenu(menu->items[idItem - 1].hSubMenu);
+            menu = menu->items[idItem - 1].submenu;
             if (menu)
                 pmbi->hwndMenu = menu->hWnd;
         }
@@ -4337,7 +4340,7 @@ BOOL MENU_SetMenu( HWND hWnd, HMENU hMenu )
 
     if (hMenu != 0)
     {
-        LPPOPUPMENU lpmenu;
+        POPUPMENU *lpmenu;
 
         if (!(lpmenu = MENU_GetMenu(hMenu))) return FALSE;
 
@@ -4373,7 +4376,7 @@ HMENU WINAPI GetSubMenu( HMENU hMenu, INT nPos )
 
     if (!(lpmi = MENU_FindItem(&menu, (UINT *)&nPos, MF_BYPOSITION))) return 0;
     if (!(lpmi->fType & MF_POPUP)) return 0;
-    return lpmi->hSubMenu;
+    return MENU_GetHandle(lpmi->submenu);
 }
 
 
@@ -4382,7 +4385,7 @@ HMENU WINAPI GetSubMenu( HMENU hMenu, INT nPos )
  */
 BOOL WINAPI DrawMenuBar( HWND hWnd )
 {
-    LPPOPUPMENU lppop;
+    POPUPMENU *lppop;
     HMENU hMenu;
 
     if (!IsWindow( hWnd ))
@@ -4410,7 +4413,7 @@ BOOL WINAPI DrawMenuBar( HWND hWnd )
  */
 DWORD WINAPI DrawMenuBarTemp(HWND hwnd, HDC hDC, LPRECT lprect, HMENU hMenu, HFONT hFont)
 {
-    LPPOPUPMENU lppop;
+    POPUPMENU *lppop;
     UINT i,retvalue;
     HFONT hfontOld = 0;
     BOOL flat_menu = FALSE;
@@ -4560,7 +4563,7 @@ HMENU WINAPI LoadMenuIndirectA( LPCVOID template )
  */
 BOOL WINAPI IsMenu(HMENU hmenu)
 {
-    LPPOPUPMENU menu = MENU_GetMenu(hmenu);
+    POPUPMENU *menu = MENU_GetMenu(hmenu);
 
     if (!menu)
     {
@@ -4662,7 +4665,7 @@ static BOOL GetMenuItemInfo_common ( HMENU hmenu, UINT item, BOOL bypos,
 	lpmii->wID = menu->wID;
 
     if (lpmii->fMask & MIIM_SUBMENU)
-	lpmii->hSubMenu = menu->hSubMenu;
+        lpmii->hSubMenu = MENU_GetHandle(menu->submenu);
     else {
         /* hSubMenu is always cleared 
          * (not on Win9x/ME ) */
@@ -4759,13 +4762,13 @@ static int MENU_depth( POPUPMENU *pmenu, int depth)
     item = pmenu->items;
     subdepth = depth;
     for( i = 0; i < pmenu->nItems && subdepth <= MAXMENUDEPTH; i++, item++){
-        POPUPMENU *psubmenu =  item->hSubMenu ? MENU_GetMenu( item->hSubMenu) : NULL;
-        if( psubmenu){
-            int bdepth = MENU_depth( psubmenu, depth);
+        if (item->submenu)
+        {
+            int bdepth = MENU_depth(item->submenu, depth);
             if( bdepth > subdepth) subdepth = bdepth;
         }
         if( subdepth > MAXMENUDEPTH)
-            TRACE("<- hmenu %p\n", item->hSubMenu);
+            TRACE("<- menu %p\n", item->submenu);
     }
     return subdepth;
 }
@@ -4804,22 +4807,30 @@ static BOOL SetMenuItemInfo_common(MENUITEM * menu,
     if (lpmii->fMask & MIIM_ID)
 	menu->wID = lpmii->wID;
 
-    if (lpmii->fMask & MIIM_SUBMENU) {
-        if ((menu->fType & MF_POPUP) && menu->hSubMenu)
-            MENU_ReleaseMenu(MENU_GetMenu(menu->hSubMenu));
-	menu->hSubMenu = lpmii->hSubMenu;
-	if (menu->hSubMenu) {
-	    POPUPMENU *subMenu = MENU_GetMenu(menu->hSubMenu);
-	    if (subMenu) {
-                if( MENU_depth( subMenu, 0) > MAXMENUDEPTH) {
+    if (lpmii->fMask & MIIM_SUBMENU)
+    {
+        if ((menu->fType & MF_POPUP) && menu->submenu)
+        {
+            MENU_ReleaseMenu(menu->submenu);
+            menu->submenu = NULL;
+        }
+        if (lpmii->hSubMenu)
+        {
+            menu->submenu = MENU_GetMenu(lpmii->hSubMenu);
+            if (menu->submenu)
+            {
+                if (MENU_depth( menu->submenu, 0) > MAXMENUDEPTH)
+                {
                     ERR( "Loop detected in menu hierarchy or maximum menu depth exceeded!\n");
-                    menu->hSubMenu = 0;
+                    menu->submenu = NULL;
                     return FALSE;
                 }
-		subMenu->wFlags |= MF_POPUP;
-		menu->fType |= MF_POPUP;
-                InterlockedIncrement(&subMenu->refcount);
-	    } else {
+                menu->submenu->wFlags |= MF_POPUP;
+                menu->fType |= MF_POPUP;
+                InterlockedIncrement(&menu->submenu->refcount);
+            }
+            else
+            {
                 SetLastError( ERROR_INVALID_PARAMETER);
                 return FALSE;
             }
@@ -5012,11 +5023,11 @@ UINT WINAPI GetMenuDefaultItem(HMENU hmenu, UINT bypos, UINT flags)
 	/* default: don't return disabled items */
 	if ( (!(GMDI_USEDISABLED & flags)) && (item->fState & MFS_DISABLED )) return -1;
 
-	/* search rekursiv when needed */
+	/* search recursively when needed */
 	if ( (item->fType & MF_POPUP) &&  (flags & GMDI_GOINTOPOPUPS) )
 	{
 	    UINT ret;
-	    ret = GetMenuDefaultItem( item->hSubMenu, bypos, flags );
+	    ret = GetMenuDefaultItem( MENU_GetHandle(item->submenu), bypos, flags );
 	    if ( -1 != ret ) return ret;
 
 	    /* when item not found in submenu, return the popup item */
@@ -5150,10 +5161,10 @@ BOOL WINAPI GetMenuItemRect(HWND hwnd, HMENU hMenu, UINT uItem, RECT *rect)
  *	actually use the items to draw the menu
  *      (recalculate and/or redraw)
  */
-static BOOL menu_SetMenuInfo( HMENU hMenu, LPCMENUINFO lpmi)
+static BOOL menu_SetMenuInfo( POPUPMENU *menu, LPCMENUINFO lpmi)
 {
-    POPUPMENU *menu;
-    if( !(menu = MENU_GetMenu(hMenu))) return FALSE;
+    if (!menu)
+        return FALSE;
 
     if (lpmi->fMask & MIM_BACKGROUND)
         menu->hbrBack = lpmi->hbrBack;
@@ -5175,7 +5186,7 @@ static BOOL menu_SetMenuInfo( HMENU hMenu, LPCMENUINFO lpmi)
         MENUITEM *item = menu->items;
         for( i = menu->nItems; i; i--, item++)
             if( item->fType & MF_POPUP)
-                menu_SetMenuInfo( item->hSubMenu, lpmi);
+                menu_SetMenuInfo( item->submenu, lpmi);
     }
     return TRUE;
 }
@@ -5183,7 +5194,7 @@ static BOOL menu_SetMenuInfo( HMENU hMenu, LPCMENUINFO lpmi)
 BOOL WINAPI SetMenuInfo (HMENU hMenu, LPCMENUINFO lpmi)
 {
     TRACE("(%p %p)\n", hMenu, lpmi);
-    if( lpmi && (lpmi->cbSize == sizeof( MENUINFO)) && (menu_SetMenuInfo( hMenu, lpmi))) {
+    if( lpmi && (lpmi->cbSize == sizeof( MENUINFO)) && (menu_SetMenuInfo( MENU_GetMenu(hMenu), lpmi))) {
 	if( lpmi->fMask & MIM_STYLE) {
 	    if (lpmi->dwStyle & MNS_AUTODISMISS) FIXME("MNS_AUTODISMISS unimplemented\n");
 	    if (lpmi->dwStyle & MNS_DRAGDROP) FIXME("MNS_DRAGDROP unimplemented\n");
@@ -5237,7 +5248,7 @@ BOOL WINAPI GetMenuInfo (HMENU hMenu, LPMENUINFO lpmi)
  */
 BOOL WINAPI SetMenuContextHelpId( HMENU hMenu, DWORD dwContextHelpID)
 {
-    LPPOPUPMENU menu;
+    POPUPMENU *menu;
 
     TRACE("(%p 0x%08x)\n", hMenu, dwContextHelpID);
 
@@ -5255,7 +5266,7 @@ BOOL WINAPI SetMenuContextHelpId( HMENU hMenu, DWORD dwContextHelpID)
  */
 DWORD WINAPI GetMenuContextHelpId( HMENU hMenu )
 {
-    LPPOPUPMENU menu;
+    POPUPMENU *menu;
 
     TRACE("(%p)\n", hMenu);
 
diff --git a/dlls/user32/tests/menu.c b/dlls/user32/tests/menu.c
index 0f6e8618a6..6a6de9de8c 100644
--- a/dlls/user32/tests/menu.c
+++ b/dlls/user32/tests/menu.c
@@ -634,9 +634,7 @@ static void test_subpopup_locked_by_menu(void)
 
     /* but TrackPopupMenu still works! */
     ret = TrackPopupMenu( hmenu, TPM_RETURNCMD, 100,100, 0, hwnd, NULL);
-    todo_wine {
-        ok( ret == itemid , "TrackPopupMenu returned %d error is %d\n", ret, GetLastError());
-    }
+    ok( ret == itemid , "TrackPopupMenu returned %#x error is %d\n", ret, GetLastError());
 
     /* clean up */
     DestroyMenu( hmenu);
-- 
2.16.3




More information about the wine-devel mailing list