[v2 PATCH] user32/menu: Return locked menu data when looking for menu items.

[Nikolay Sivov]

Signed-off-by: Nikolay Sivov 
---

This hopefully addresses comments I got for original patch. It takes
handle on input, and returns locked menu data and item pointer in a single
structure.

dlls/user32/menu.c | 566 ++++++++++++++++++++++++++++-----------------
 1 file changed, 359 insertions(+), 207 deletions(-)

diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c
index 537c2ebe1a..5778af2a0a 100644
--- a/dlls/user32/menu.c
+++ b/dlls/user32/menu.c
@@ -584,7 +584,12 @@ static UINT  MENU_GetStartOfPrevColumn(
     return i;
 }

-
+struct menu_item_desc
+{
+    MENUITEM *item;
+    POPUPMENU *menu;
+    UINT id;
+};

/***********************************************************************
  *           MENU_FindItem
@@ -592,18 +597,29 @@ static UINT  MENU_GetStartOfPrevColumn(
  * Find a menu item. Return a pointer on the item, and modifies *hmenu
  * in case the item was in a sub-menu.
  */
-static MENUITEM *MENU_FindItem( HMENU *hmenu, UINT *nPos, UINT wFlags )
+static BOOL MENU_FindItem(HMENU hmenu, UINT wFlags, struct menu_item_desc *desc)
 {
+    UINT fallback_pos = ~0u, i;
     POPUPMENU *menu;
-    MENUITEM *fallback = NULL;
-    UINT fallback_pos = 0;
-    UINT i;

-    if ((*hmenu == (HMENU)0xffff) || (!(menu = MENU_GetMenu(*hmenu)))) return NULL;
+    desc->menu = NULL;
+    desc->item = NULL;
+
+    menu = grab_menu_ptr(hmenu);
+    if (!menu)
+        return FALSE;
+
     if (wFlags & MF_BYPOSITION)
     {
-	if (*nPos >= menu->nItems) return NULL;
-	return &menu->items[*nPos];
+        if (desc->id >= menu->nItems)
+        {
+            release_menu_ptr(menu);
+            return FALSE;
+        }
+
+        desc->menu = menu;
+        desc->item = &menu->items[desc->id];
+        return TRUE;
     }
     else
     {
@@ -612,32 +628,37 @@ static MENUITEM *MENU_FindItem( HMENU *hmenu, UINT *nPos, UINT wFlags )
    {
        if (item->fType & MF_POPUP)
        {
-		HMENU hsubmenu = item->hSubMenu;
-		MENUITEM *subitem = MENU_FindItem( &hsubmenu, nPos, wFlags );
-		if (subitem)
-		{
-		    *hmenu = hsubmenu;
-		    return subitem;
-		}
-		else if (item->wID == *nPos)
+                if (MENU_FindItem(item->hSubMenu, wFlags, desc))
+                {
+                    release_menu_ptr(menu);
+                    return TRUE;
+                }
+                else if (item->wID == desc->id)
    	{
    	    /* fallback to this item if nothing else found */
    	    fallback_pos = i;
-		    fallback = item;
    	}
        }
-	    else if (item->wID == *nPos)
+	    else if (item->wID == desc->id)
        {
-		*nPos = i;
-		return item;
+                desc->menu = menu;
+                desc->item = item;
+                desc->id = i;
+                return TRUE;
        }
    }
     }

-    if (fallback)
-        *nPos = fallback_pos;
+    if (fallback_pos != ~0u)
+    {
+        desc->menu = menu;
+        desc->item = &menu->items[fallback_pos];
+        desc->id = fallback_pos;
+    }
+    else
+        release_menu_ptr(menu);

-    return fallback;
+    return fallback_pos != ~0u;
 }

/***********************************************************************
@@ -2135,28 +2156,36 @@ static void MENU_MoveSelection( HWND hwndOwner, HMENU hmenu, INT offset )
  *
  * Insert (allocate) a new item into a menu.
  */
-static MENUITEM *MENU_InsertItem( HMENU hMenu, UINT pos, UINT flags )
+static BOOL MENU_InsertItem(HMENU hMenu, UINT flags, struct menu_item_desc *desc)
 {
+    UINT pos = desc->id;
     MENUITEM *newItems;
     POPUPMENU *menu;

-    if (!(menu = MENU_GetMenu(hMenu)))
-        return NULL;
+    desc->menu = NULL;
+    desc->item = NULL;

/* Find where to insert new item */
-
     if (flags & MF_BYPOSITION) {
-        if (pos > menu->nItems)
+        menu = grab_menu_ptr(hMenu);
+        if (desc->id > menu->nItems)
             pos = menu->nItems;
     } else {
-        if (!MENU_FindItem( &hMenu, &pos, flags ))
+        struct menu_item_desc item_desc;
+
+        item_desc.id = desc->id;
+        if (!MENU_FindItem(hMenu, flags, &item_desc))
+        {
+            menu = grab_menu_ptr(hMenu);
             pos = menu->nItems;
-        else {
-            if (!(menu = MENU_GetMenu( hMenu )))
-                return NULL;
         }
+        else
+            menu = item_desc.menu;
     }

+    if (!menu)
+        return FALSE;
+
     /* Make sure that MDI system buttons stay on the right side.
      * Note: XP treats only bitmap handles 1 - 6 as "magic" ones
      * regardless of their id.
@@ -2172,8 +2201,9 @@ static MENUITEM *MENU_InsertItem( HMENU hMenu, UINT pos, UINT flags )
     newItems = HeapAlloc( GetProcessHeap(), 0, sizeof(MENUITEM) * (menu->nItems+1) );
     if (!newItems)
     {
+        release_menu_ptr(menu);
         WARN("allocation failed\n" );
-        return NULL;
+        return FALSE;
     }
     if (menu->nItems > 0)
     {
@@ -2187,7 +2217,12 @@ static MENUITEM *MENU_InsertItem( HMENU hMenu, UINT pos, UINT flags )
     menu->nItems++;
     memset( &newItems[pos], 0, sizeof(*newItems) );
     menu->Height = 0; /* force size recalculate */
-    return &newItems[pos];
+
+    desc->menu = menu;
+    desc->id = pos;
+    desc->item = &newItems[pos];
+
+    return TRUE;
 }

@@ -3706,13 +3741,15 @@ BOOL WINAPI ChangeMenuW( HMENU hMenu, UINT pos, LPCWSTR data,
  */
 DWORD WINAPI CheckMenuItem( HMENU hMenu, UINT id, UINT flags )
 {
-    MENUITEM *item;
+    struct menu_item_desc item_desc;
     DWORD ret;

-    if (!(item = MENU_FindItem( &hMenu, &id, flags ))) return -1;
-    ret = item->fState & MF_CHECKED;
-    if (flags & MF_CHECKED) item->fState |= MF_CHECKED;
-    else item->fState &= ~MF_CHECKED;
+    item_desc.id = id;
+    if (!MENU_FindItem(hMenu, flags, &item_desc)) return -1;
+    ret = item_desc.item->fState & MF_CHECKED;
+    if (flags & MF_CHECKED) item_desc.item->fState |= MF_CHECKED;
+    else item_desc.item->fState &= ~MF_CHECKED;
+    release_menu_ptr(item_desc.menu);
     return ret;
 }

@@ -3722,40 +3759,45 @@ DWORD WINAPI CheckMenuItem( HMENU hMenu, UINT id, UINT flags )
  */
 BOOL WINAPI EnableMenuItem( HMENU hMenu, UINT wItemID, UINT wFlags )
 {
+    struct menu_item_desc item_desc;
     UINT    oldflags;
-    MENUITEM *item;
-    POPUPMENU *menu;

TRACE("(%p, %04x, %04x) !\n", hMenu, wItemID, wFlags);

/* Get the Popupmenu to access the owner menu */
-    if (!(menu = MENU_GetMenu(hMenu)))
-	return (UINT)-1;
-
-    if (!(item = MENU_FindItem( &hMenu, &wItemID, wFlags )))
+    item_desc.id = wItemID;
+    if (!MENU_FindItem(hMenu, wFlags, &item_desc))
    return (UINT)-1;

-    oldflags = item->fState & (MF_GRAYED | MF_DISABLED);
-    item->fState ^= (oldflags ^ wFlags) & (MF_GRAYED | MF_DISABLED);
+    oldflags = item_desc.item->fState & (MF_GRAYED | MF_DISABLED);
+    item_desc.item->fState ^= (oldflags ^ wFlags) & (MF_GRAYED | MF_DISABLED);

/* If the close item in the system menu change update the close button */
-    if((item->wID == SC_CLOSE) && (oldflags != wFlags))
+    if ((item_desc.item->wID == SC_CLOSE) && (oldflags != wFlags))
     {
-	if (menu->hSysMenuOwner != 0)
+        if (item_desc.menu->hSysMenuOwner)
    {
             RECT rc;
        POPUPMENU* parentMenu;
+            HWND hwnd;

/* Get the parent menu to access*/
-	    if (!(parentMenu = MENU_GetMenu(menu->hSysMenuOwner)))
-		return (UINT)-1;
+            parentMenu = grab_menu_ptr(item_desc.menu->hSysMenuOwner);
+            release_menu_ptr(item_desc.menu);
+            if (!parentMenu)
+                return (UINT)-1;
+
+            hwnd = parentMenu->hWnd;
+            release_menu_ptr(parentMenu);

/* Refresh the frame to reflect the change */
-            WIN_GetRectangles( parentMenu->hWnd, COORDS_CLIENT, &rc, NULL );
+            WIN_GetRectangles( hwnd, COORDS_CLIENT, &rc, NULL );
             rc.bottom = 0;
-            RedrawWindow(parentMenu->hWnd, &rc, 0, RDW_FRAME | RDW_INVALIDATE | RDW_NOCHILDREN);
+            RedrawWindow(hwnd, &rc, 0, RDW_FRAME | RDW_INVALIDATE | RDW_NOCHILDREN);
    }
     }
+    else
+        release_menu_ptr(item_desc.menu);

return oldflags;
 }
@@ -3770,21 +3812,34 @@ INT WINAPI GetMenuStringA(
    LPSTR str,	/* [out] outbuffer. If NULL, func returns entry length*/
    INT nMaxSiz,	/* [in] length of buffer. if 0, func returns entry len*/
    UINT wFlags	/* [in] MF_ flags */
-) {
-    MENUITEM *item;
+)
+{
+    struct menu_item_desc item_desc;
+    INT ret;

TRACE("menu=%p item=%04x ptr=%p len=%d flags=%04x\n", hMenu, wItemID, str, nMaxSiz, wFlags );
     if (str && nMaxSiz) str[0] = '\0';
-    if (!(item = MENU_FindItem( &hMenu, &wItemID, wFlags ))) {
+
+    item_desc.id = wItemID;
+    if (!MENU_FindItem(hMenu, wFlags, &item_desc)) {
         SetLastError( ERROR_MENU_ITEM_NOT_FOUND);
         return 0;
     }
-    if (!item->text) return 0;
-    if (!str || !nMaxSiz) return WideCharToMultiByte( CP_ACP, 0, item->text, -1, NULL, 0, NULL, NULL );
-    if (!WideCharToMultiByte( CP_ACP, 0, item->text, -1, str, nMaxSiz, NULL, NULL ))
-        str[nMaxSiz-1] = 0;
+
+    if (!item_desc.item->text)
+        ret = 0;
+    else if (!str || !nMaxSiz)
+        ret = WideCharToMultiByte( CP_ACP, 0, item_desc.item->text, -1, NULL, 0, NULL, NULL );
+    else
+    {
+        if (!WideCharToMultiByte( CP_ACP, 0, item_desc.item->text, -1, str, nMaxSiz, NULL, NULL ))
+            str[nMaxSiz-1] = 0;
+        ret = strlen(str);
+    }
+    release_menu_ptr(item_desc.menu);
+
     TRACE("returning %s\n", debugstr_a(str));
-    return strlen(str);
+    return ret;
 }

@@ -3794,22 +3849,33 @@ INT WINAPI GetMenuStringA(
 INT WINAPI GetMenuStringW( HMENU hMenu, UINT wItemID,
                                LPWSTR str, INT nMaxSiz, UINT wFlags )
 {
-    MENUITEM *item;
+    struct menu_item_desc item_desc;
+    INT ret;

TRACE("menu=%p item=%04x ptr=%p len=%d flags=%04x\n", hMenu, wItemID, str, nMaxSiz, wFlags );
     if (str && nMaxSiz) str[0] = '\0';
-    if (!(item = MENU_FindItem( &hMenu, &wItemID, wFlags ))) {
+
+    item_desc.id = wItemID;
+    if (!MENU_FindItem(hMenu, wFlags, &item_desc)) {
         SetLastError( ERROR_MENU_ITEM_NOT_FOUND);
         return 0;
     }
-    if (!str || !nMaxSiz) return item->text ? strlenW(item->text) : 0;
-    if( !(item->text)) {
+    if (!str || !nMaxSiz)
+        ret = item_desc.item->text ? strlenW(item_desc.item->text) : 0;
+    else if (!item_desc.item->text)
+    {
         str[0] = 0;
-        return 0;
+        ret = 0;
     }
-    lstrcpynW( str, item->text, nMaxSiz );
+    else
+    {
+        lstrcpynW( str, item_desc.item->text, nMaxSiz );
+        ret = strlenW(str);
+    }
+    release_menu_ptr(item_desc.menu);
+
     TRACE("returning %s\n", debugstr_w(str));
-    return strlenW(str);
+    return ret;
 }

@@ -3819,13 +3885,19 @@ INT WINAPI GetMenuStringW( HMENU hMenu, UINT wItemID,
 BOOL WINAPI HiliteMenuItem( HWND hWnd, HMENU hMenu, UINT wItemID,
                                 UINT wHilite )
 {
-    LPPOPUPMENU menu;
+    struct menu_item_desc item_desc;
+
     TRACE("(%p, %p, %04x, %04x);\n", hWnd, hMenu, wItemID, wHilite);
-    if (!MENU_FindItem( &hMenu, &wItemID, wHilite )) return FALSE;
-    if (!(menu = MENU_GetMenu(hMenu))) return FALSE;
-    if (menu->FocusedItem == wItemID) return TRUE;
-    MENU_HideSubPopups( hWnd, hMenu, FALSE, 0 );
-    MENU_SelectItem( hWnd, hMenu, wItemID, TRUE, 0 );
+
+    item_desc.id = wItemID;
+    if (!MENU_FindItem(hMenu, wHilite, &item_desc)) return FALSE;
+
+    if (item_desc.menu->FocusedItem != item_desc.id)
+    {
+        MENU_HideSubPopups( hWnd, item_desc.menu->obj.handle, FALSE, 0 );
+        MENU_SelectItem( hWnd, item_desc.menu->obj.handle, item_desc.id, TRUE, 0 );
+    }
+    release_menu_ptr(item_desc.menu);
     return TRUE;
 }

@@ -3835,23 +3907,32 @@ BOOL WINAPI HiliteMenuItem( HWND hWnd, HMENU hMenu, UINT wItemID,
  */
 UINT WINAPI GetMenuState( HMENU hMenu, UINT wItemID, UINT wFlags )
 {
-    MENUITEM *item;
+    struct menu_item_desc item_desc;
+    UINT state;
+
     TRACE("(menu=%p, id=%04x, flags=%04x);\n", hMenu, wItemID, wFlags);
-    if (!(item = MENU_FindItem( &hMenu, &wItemID, wFlags ))) return -1;
-    debug_print_menuitem ("  item: ", item, "");
-    if (item->fType & MF_POPUP)
+
+    item_desc.id = wItemID;
+    if (!MENU_FindItem(hMenu, wFlags, &item_desc)) return -1;
+    debug_print_menuitem ("  item: ", item_desc.item, "");
+    if (item_desc.item->fType & MF_POPUP)
     {
-	POPUPMENU *menu = MENU_GetMenu( item->hSubMenu );
-	if (!menu) return -1;
-	else return (menu->nItems << 8) | ((item->fState|item->fType) & 0xff);
+        POPUPMENU *submenu = grab_menu_ptr(item_desc.item->hSubMenu);
+        if (submenu)
+            state = (submenu->nItems << 8) | ((item_desc.item->fState | item_desc.item->fType) & 0xff);
+        else
+            state = -1;
+        release_menu_ptr(submenu);
     }
     else
     {
    /* We used to (from way back then) mask the result to 0xff.  */
    /* I don't know why and it seems wrong as the documented */
    /* return flag MF_SEPARATOR is outside that mask.  */
-	return (item->fType | item->fState);
+        state = (item_desc.item->fType | item_desc.item->fState);
     }
+    release_menu_ptr(item_desc.menu);
+    return state;
 }

@@ -3877,12 +3958,15 @@ INT WINAPI GetMenuItemCount( HMENU hMenu )
  */
 UINT WINAPI GetMenuItemID( HMENU hMenu, INT nPos )
 {
-    MENUITEM * lpmi;
+    struct menu_item_desc item_desc;
+    UINT id;

-    if (!(lpmi = MENU_FindItem(&hMenu,(UINT*)&nPos,MF_BYPOSITION))) return -1;
-    if (lpmi->fType & MF_POPUP) return -1;
-    return lpmi->wID;
+    item_desc.id = nPos;
+    if (!MENU_FindItem(hMenu, MF_BYPOSITION, &item_desc)) return -1;

+    id = item_desc.item->fType & MF_POPUP ? -1 : item_desc.item->wID;
+    release_menu_ptr(item_desc.menu);
+    return id;
 }

@@ -3935,8 +4019,9 @@ static void MENU_mnu2mnuii( UINT flags, UINT_PTR id, LPCWSTR str,
 BOOL WINAPI InsertMenuW( HMENU hMenu, UINT pos, UINT flags,
                          UINT_PTR id, LPCWSTR str )
 {
-    MENUITEM *item;
+    struct menu_item_desc item_desc;
     MENUITEMINFOW mii;
+    BOOL ret;

if (IS_STRING_ITEM(flags) && str)
         TRACE("hMenu %p, pos %d, flags %08x, id %04lx, str %s\n",
@@ -3944,16 +4029,18 @@ BOOL WINAPI InsertMenuW( HMENU hMenu, UINT pos, UINT flags,
     else TRACE("hMenu %p, pos %d, flags %08x, id %04lx, str %p (not a string)\n",
                hMenu, pos, flags, id, str );

-    if (!(item = MENU_InsertItem( hMenu, pos, flags ))) return FALSE;
+    item_desc.id = pos;
+    if (!MENU_InsertItem(hMenu, flags, &item_desc)) return FALSE;
     MENU_mnu2mnuii( flags, id, str, &mii);
-    if (!(SetMenuItemInfo_common( item, &mii, TRUE)))
-    {
+
+    ret = SetMenuItemInfo_common( item_desc.item, &mii, TRUE);
+    if (ret)
+        item_desc.item->hCheckBit = item_desc.item->hUnCheckBit = 0;
+    else
         RemoveMenu( hMenu, pos, flags );
-        return FALSE;
-    }
+    release_menu_ptr(item_desc.menu);

-    item->hCheckBit = item->hUnCheckBit = 0;
-    return TRUE;
+    return ret;
 }

@@ -4006,35 +4093,38 @@ BOOL WINAPI AppendMenuW( HMENU hMenu, UINT flags,
  */
 BOOL WINAPI RemoveMenu( HMENU hMenu, UINT nPos, UINT wFlags )
 {
-    LPPOPUPMENU	menu;
-    MENUITEM *item;
+    struct menu_item_desc item_desc;

TRACE("(menu=%p pos=%04x flags=%04x)\n",hMenu, nPos, wFlags);
-    if (!(item = MENU_FindItem( &hMenu, &nPos, wFlags ))) return FALSE;
-    if (!(menu = MENU_GetMenu(hMenu))) return FALSE;

-      /* Remove item */
+    item_desc.id = nPos;
+    if (!MENU_FindItem(hMenu, wFlags, &item_desc))
+        return FALSE;

-    MENU_FreeItemData( item );
+    /* Remove item */
+    MENU_FreeItemData( item_desc.item );

-    if (--menu->nItems == 0)
+    if (--item_desc.menu->nItems == 0)
     {
-        HeapFree( GetProcessHeap(), 0, menu->items );
-        menu->items = NULL;
+        HeapFree( GetProcessHeap(), 0, item_desc.menu->items );
+        item_desc.menu->items = NULL;
     }
     else
     {
-        MENUITEM *new_items;
-	while(nPos < menu->nItems)
+        MENUITEM *new_items, *item = item_desc.item;
+
+	while (nPos < item_desc.menu->nItems)
    {
        *item = *(item+1);
        item++;
        nPos++;
    }
-        new_items = HeapReAlloc( GetProcessHeap(), 0, menu->items,
-                                 menu->nItems * sizeof(MENUITEM) );
-        if (new_items) menu->items = new_items;
+        new_items = HeapReAlloc( GetProcessHeap(), 0, item_desc.menu->items,
+                                 item_desc.menu->nItems * sizeof(MENUITEM) );
+        if (new_items) item_desc.menu->items = new_items;
     }
+    release_menu_ptr(item_desc.menu);
+
     return TRUE;
 }

@@ -4044,11 +4134,18 @@ BOOL WINAPI RemoveMenu( HMENU hMenu, UINT nPos, UINT wFlags )
  */
 BOOL WINAPI DeleteMenu( HMENU hMenu, UINT nPos, UINT wFlags )
 {
-    MENUITEM *item = MENU_FindItem( &hMenu, &nPos, wFlags );
-    if (!item) return FALSE;
-    if (item->fType & MF_POPUP) DestroyMenu( item->hSubMenu );
-      /* nPos is now the position of the item */
-    RemoveMenu( hMenu, nPos, wFlags | MF_BYPOSITION );
+    struct menu_item_desc item_desc;
+
+    item_desc.id = nPos;
+    if (!MENU_FindItem(hMenu, wFlags, &item_desc))
+        return FALSE;
+
+    if (item_desc.item->fType & MF_POPUP)
+        DestroyMenu(item_desc.item->hSubMenu);
+
+    /* nPos is now the position of the item */
+    RemoveMenu(item_desc.menu->obj.handle, item_desc.id, wFlags | MF_BYPOSITION);
+    release_menu_ptr(item_desc.menu);
     return TRUE;
 }

@@ -4059,23 +4156,27 @@ BOOL WINAPI DeleteMenu( HMENU hMenu, UINT nPos, UINT wFlags )
 BOOL WINAPI ModifyMenuW( HMENU hMenu, UINT pos, UINT flags,
                          UINT_PTR id, LPCWSTR str )
 {
-    MENUITEM *item;
+    struct menu_item_desc item_desc;
     MENUITEMINFOW mii;
+    BOOL ret;

if (IS_STRING_ITEM(flags))
         TRACE("%p %d %04x %04lx %s\n", hMenu, pos, flags, id, debugstr_w(str) );
     else
         TRACE("%p %d %04x %04lx %p\n", hMenu, pos, flags, id, str );

-    if (!(item = MENU_FindItem( &hMenu, &pos, flags )))
+    item_desc.id = pos;
+    if (!MENU_FindItem(hMenu, flags, &item_desc))
     {
         /* workaround for Word 95: pretend that SC_TASKLIST item exists */
         if (pos == SC_TASKLIST && !(flags & MF_BYPOSITION)) return TRUE;
         return FALSE;
     }
-    MENU_GetMenu(hMenu)->Height = 0; /* force size recalculate */
+    item_desc.menu->Height = 0; /* force size recalculate */
     MENU_mnu2mnuii( flags, id, str, &mii);
-    return SetMenuItemInfo_common( item, &mii, TRUE);
+    ret = SetMenuItemInfo_common(item_desc.item, &mii, TRUE);
+    release_menu_ptr(item_desc.menu);
+    return ret;
 }

@@ -4134,20 +4235,23 @@ DWORD WINAPI GetMenuCheckMarkDimensions(void)
 BOOL WINAPI SetMenuItemBitmaps( HMENU hMenu, UINT nPos, UINT wFlags,
                                     HBITMAP hNewUnCheck, HBITMAP hNewCheck)
 {
-    MENUITEM *item;
+    struct menu_item_desc item_desc;

-    if (!(item = MENU_FindItem( &hMenu, &nPos, wFlags ))) return FALSE;
+    item_desc.id = nPos;
+    if (!MENU_FindItem(hMenu, wFlags, &item_desc)) return FALSE;

if (!hNewCheck && !hNewUnCheck)
     {
-	item->fState &= ~MF_USECHECKBITMAPS;
+        item_desc.item->fState &= ~MF_USECHECKBITMAPS;
     }
     else  /* Install new bitmaps */
     {
-	item->hCheckBit = hNewCheck;
-	item->hUnCheckBit = hNewUnCheck;
-	item->fState |= MF_USECHECKBITMAPS;
+        item_desc.item->hCheckBit = hNewCheck;
+        item_desc.item->hUnCheckBit = hNewUnCheck;
+        item_desc.item->fState |= MF_USECHECKBITMAPS;
     }
+    release_menu_ptr(item_desc.menu);
+
     return TRUE;
 }

@@ -4418,11 +4522,19 @@ BOOL WINAPI SetMenu( HWND hWnd, HMENU hMenu )
  */
 HMENU WINAPI GetSubMenu( HMENU hMenu, INT nPos )
 {
-    MENUITEM * lpmi;
+    struct menu_item_desc item_desc;
+    HMENU submenu;
+
+    item_desc.id = nPos;
+    if (!MENU_FindItem(hMenu, MF_BYPOSITION, &item_desc)) return 0;
+
+    if (item_desc.item->fType & MF_POPUP)
+        submenu = item_desc.item->hSubMenu;
+    else
+        submenu = 0;

-    if (!(lpmi = MENU_FindItem(&hMenu,(UINT*)&nPos,MF_BYPOSITION))) return 0;
-    if (!(lpmi->fType & MF_POPUP)) return 0;
-    return lpmi->hSubMenu;
+    release_menu_ptr(item_desc.menu);
+    return submenu;
 }

@@ -4634,28 +4746,33 @@ BOOL WINAPI IsMenu(HMENU hmenu)
 static BOOL GetMenuItemInfo_common ( HMENU hmenu, UINT item, BOOL bypos,
    				LPMENUITEMINFOW lpmii, BOOL unicode)
 {
-    MENUITEM *menu = MENU_FindItem (&hmenu, &item, bypos ? MF_BYPOSITION : 0);
+    struct menu_item_desc item_desc;

-    debug_print_menuitem("GetMenuItemInfo_common: ", menu, "");
+    item_desc.id = item;
+    MENU_FindItem(hmenu, bypos ? MF_BYPOSITION : 0, &item_desc);

-    if (!menu) {
+    debug_print_menuitem("GetMenuItemInfo_common: ", item_desc.item, "");
+
+    if (!item_desc.item)
+    {
         SetLastError( ERROR_MENU_ITEM_NOT_FOUND);
         return FALSE;
     }

if( lpmii->fMask & MIIM_TYPE) {
         if( lpmii->fMask & ( MIIM_STRING | MIIM_FTYPE | MIIM_BITMAP)) {
+            release_menu_ptr(item_desc.menu);
             WARN("invalid combination of fMask bits used\n");
             /* this does not happen on Win9x/ME */
             SetLastError( ERROR_INVALID_PARAMETER);
             return FALSE;
         }
-	lpmii->fType = menu->fType & MENUITEMINFO_TYPE_MASK;
-        if (menu->hbmpItem && !IS_MAGIC_BITMAP(menu->hbmpItem))
+        lpmii->fType = item_desc.item->fType & MENUITEMINFO_TYPE_MASK;
+        if (item_desc.item->hbmpItem && !IS_MAGIC_BITMAP(item_desc.item->hbmpItem))
             lpmii->fType |= MFT_BITMAP;
-	lpmii->hbmpItem = menu->hbmpItem; /* not on Win9x/ME */
+        lpmii->hbmpItem = item_desc.item->hbmpItem; /* not on Win9x/ME */
         if( lpmii->fType & MFT_BITMAP) {
-	    lpmii->dwTypeData = (LPWSTR) menu->hbmpItem;
+	    lpmii->dwTypeData = (LPWSTR) item_desc.item->hbmpItem;
        lpmii->cch = 0;
         } else if( lpmii->fType & (MFT_OWNERDRAW | MFT_SEPARATOR)) {
             /* this does not happen on Win9x/ME */
@@ -4666,7 +4783,7 @@ static BOOL GetMenuItemInfo_common ( HMENU hmenu, UINT item, BOOL bypos,

/* copy the text string */
     if ((lpmii->fMask & (MIIM_TYPE|MIIM_STRING))) {
-         if( !menu->text ) {
+         if (!item_desc.item->text) {
                 if(lpmii->dwTypeData && lpmii->cch) {
                     if( unicode)
                         *((WCHAR *)lpmii->dwTypeData) = 0;
@@ -4678,16 +4795,16 @@ static BOOL GetMenuItemInfo_common ( HMENU hmenu, UINT item, BOOL bypos,
             int len;
             if (unicode)
             {
-                len = strlenW(menu->text);
+                len = strlenW(item_desc.item->text);
                 if(lpmii->dwTypeData && lpmii->cch)
-                    lstrcpynW(lpmii->dwTypeData, menu->text, lpmii->cch);
+                    lstrcpynW(lpmii->dwTypeData, item_desc.item->text, lpmii->cch);
             }
             else
             {
-                len = WideCharToMultiByte( CP_ACP, 0, menu->text, -1, NULL,
+                len = WideCharToMultiByte( CP_ACP, 0, item_desc.item->text, -1, NULL,
                         0, NULL, NULL ) - 1;
                 if(lpmii->dwTypeData && lpmii->cch)
-                    if (!WideCharToMultiByte( CP_ACP, 0, menu->text, -1,
+                    if (!WideCharToMultiByte( CP_ACP, 0, item_desc.item->text, -1,
                             (LPSTR)lpmii->dwTypeData, lpmii->cch, NULL, NULL ))
                         ((LPSTR)lpmii->dwTypeData)[lpmii->cch - 1] = 0;
             }
@@ -4706,19 +4823,19 @@ static BOOL GetMenuItemInfo_common ( HMENU hmenu, UINT item, BOOL bypos,
     }

if (lpmii->fMask & MIIM_FTYPE)
-	lpmii->fType = menu->fType & MENUITEMINFO_TYPE_MASK;
+        lpmii->fType = item_desc.item->fType & MENUITEMINFO_TYPE_MASK;

if (lpmii->fMask & MIIM_BITMAP)
-	lpmii->hbmpItem = menu->hbmpItem;
+        lpmii->hbmpItem = item_desc.item->hbmpItem;

if (lpmii->fMask & MIIM_STATE)
-	lpmii->fState = menu->fState & MENUITEMINFO_STATE_MASK;
+        lpmii->fState = item_desc.item->fState & MENUITEMINFO_STATE_MASK;

if (lpmii->fMask & MIIM_ID)
-	lpmii->wID = menu->wID;
+        lpmii->wID = item_desc.item->wID;

if (lpmii->fMask & MIIM_SUBMENU)
-	lpmii->hSubMenu = menu->hSubMenu;
+        lpmii->hSubMenu = item_desc.item->hSubMenu;
     else {
         /* hSubMenu is always cleared 
          * (not on Win9x/ME ) */
@@ -4726,13 +4843,14 @@ static BOOL GetMenuItemInfo_common ( HMENU hmenu, UINT item, BOOL bypos,
     }

if (lpmii->fMask & MIIM_CHECKMARKS) {
-	lpmii->hbmpChecked = menu->hCheckBit;
-	lpmii->hbmpUnchecked = menu->hUnCheckBit;
+        lpmii->hbmpChecked = item_desc.item->hCheckBit;
+        lpmii->hbmpUnchecked = item_desc.item->hUnCheckBit;
     }
     if (lpmii->fMask & MIIM_DATA)
-	lpmii->dwItemData = menu->dwItemData;
+        lpmii->dwItemData = item_desc.item->dwItemData;

-  return TRUE;
+    release_menu_ptr(item_desc.menu);
+    return TRUE;
 }

/**********************************************************************
@@ -4949,20 +5067,24 @@ static BOOL MENU_NormalizeMenuItemInfoStruct( const MENUITEMINFOW *pmii_in,
 BOOL WINAPI SetMenuItemInfoA(HMENU hmenu, UINT item, BOOL bypos,
                                  const MENUITEMINFOA *lpmii)
 {
-    MENUITEM *menuitem;
+    struct menu_item_desc item_desc;
     MENUITEMINFOW mii;
+    BOOL ret;

TRACE("hmenu %p, item %u, by pos %d, info %p\n", hmenu, item, bypos, lpmii);

if (!MENU_NormalizeMenuItemInfoStruct( (const MENUITEMINFOW *)lpmii, &mii )) return FALSE;

-    if (!(menuitem = MENU_FindItem( &hmenu, &item, bypos? MF_BYPOSITION : 0 )))
+    item_desc.id = item;
+    if (!MENU_FindItem(hmenu, bypos ? MF_BYPOSITION : 0, &item_desc))
     {
         /* workaround for Word 95: pretend that SC_TASKLIST item exists */
         if (item == SC_TASKLIST && !bypos) return TRUE;
         return FALSE;
     }
-    return SetMenuItemInfo_common( menuitem, &mii, FALSE );
+    ret = SetMenuItemInfo_common(item_desc.item, &mii, FALSE);
+    release_menu_ptr(item_desc.menu);
+    return ret;
 }

/**********************************************************************
@@ -4971,19 +5093,25 @@ BOOL WINAPI SetMenuItemInfoA(HMENU hmenu, UINT item, BOOL bypos,
 BOOL WINAPI SetMenuItemInfoW(HMENU hmenu, UINT item, BOOL bypos,
                                  const MENUITEMINFOW *lpmii)
 {
-    MENUITEM *menuitem;
+    struct menu_item_desc item_desc;
     MENUITEMINFOW mii;
+    BOOL ret;

TRACE("hmenu %p, item %u, by pos %d, info %p\n", hmenu, item, bypos, lpmii);

if (!MENU_NormalizeMenuItemInfoStruct( lpmii, &mii )) return FALSE;
-    if (!(menuitem = MENU_FindItem( &hmenu, &item, bypos? MF_BYPOSITION : 0 )))
+
+    item_desc.id = item;
+    if (!MENU_FindItem(hmenu, bypos ? MF_BYPOSITION : 0, &item_desc))
     {
         /* workaround for Word 95: pretend that SC_TASKLIST item exists */
         if (item == SC_TASKLIST && !bypos) return TRUE;
         return FALSE;
     }
-    return SetMenuItemInfo_common( menuitem, &mii, TRUE );
+
+    ret = SetMenuItemInfo_common(item_desc.item, &mii, TRUE);
+    release_menu_ptr(item_desc.menu);
+    return ret;
 }

static BOOL set_menu_default_item(POPUPMENU *menu, UINT uItem, UINT bypos)
@@ -5090,15 +5218,21 @@ UINT WINAPI GetMenuDefaultItem(HMENU hmenu, UINT bypos, UINT flags)
 BOOL WINAPI InsertMenuItemA(HMENU hMenu, UINT uItem, BOOL bypos,
                                 const MENUITEMINFOA *lpmii)
 {
-    MENUITEM *item;
+    struct menu_item_desc item_desc;
     MENUITEMINFOW mii;
+    BOOL ret;

TRACE("hmenu %p, item %04x, by pos %d, info %p\n", hMenu, uItem, bypos, lpmii);

if (!MENU_NormalizeMenuItemInfoStruct( (const MENUITEMINFOW *)lpmii, &mii )) return FALSE;

-    item = MENU_InsertItem(hMenu, uItem, bypos ? MF_BYPOSITION : 0 );
-    return SetMenuItemInfo_common(item, &mii, FALSE);
+    item_desc.id = uItem;
+    if (!MENU_InsertItem(hMenu, bypos ? MF_BYPOSITION : 0, &item_desc))
+        return FALSE;
+
+    ret = SetMenuItemInfo_common(item_desc.item, &mii, FALSE);
+    release_menu_ptr(item_desc.menu);
+    return ret;
 }

@@ -5108,64 +5242,74 @@ BOOL WINAPI InsertMenuItemA(HMENU hMenu, UINT uItem, BOOL bypos,
 BOOL WINAPI InsertMenuItemW(HMENU hMenu, UINT uItem, BOOL bypos,
                                 const MENUITEMINFOW *lpmii)
 {
-    MENUITEM *item;
+    struct menu_item_desc item_desc;
     MENUITEMINFOW mii;
+    BOOL ret;

TRACE("hmenu %p, item %04x, by pos %d, info %p\n", hMenu, uItem, bypos, lpmii);

if (!MENU_NormalizeMenuItemInfoStruct( lpmii, &mii )) return FALSE;

-    item = MENU_InsertItem(hMenu, uItem, bypos ? MF_BYPOSITION : 0 );
-    return SetMenuItemInfo_common(item, &mii, TRUE);
+    item_desc.id = uItem;
+    if (!MENU_InsertItem(hMenu, bypos ? MF_BYPOSITION : 0, &item_desc))
+        return FALSE;
+
+    ret = SetMenuItemInfo_common(item_desc.item, &mii, TRUE);
+    release_menu_ptr(item_desc.menu);
+    return ret;
 }

/**********************************************************************
  *		CheckMenuRadioItem    (USER32.@)
  */

-BOOL WINAPI CheckMenuRadioItem(HMENU hMenu,
-				   UINT first, UINT last, UINT check,
-				   UINT bypos)
+BOOL WINAPI CheckMenuRadioItem(HMENU hMenu, UINT first, UINT last,
+    UINT check, UINT flags)
 {
+    struct menu_item_desc first_item = { 0 }, check_item = { 0 };
     BOOL done = FALSE;
     UINT i;
-    MENUITEM *mi_first = NULL, *mi_check;
-    HMENU m_first, m_check;

for (i = first; i <= last; i++)
     {
-        UINT pos = i;
-
-        if (!mi_first)
+        if (!first_item.menu)
         {
-            m_first = hMenu;
-            mi_first = MENU_FindItem(&m_first, &pos, bypos);
-            if (!mi_first) continue;
-            mi_check = mi_first;
-            m_check = m_first;
+            first_item.id = i;
+            if (!MENU_FindItem(hMenu, flags, &first_item)) continue;
+
+            check_item = first_item;
+            check_item.menu = grab_menu_ptr(first_item.menu->obj.handle);
         }
         else
         {
-            m_check = hMenu;
-            mi_check = MENU_FindItem(&m_check, &pos, bypos);
-            if (!mi_check) continue;
+            check_item.id = i;
+            if (!MENU_FindItem(hMenu, flags, &check_item)) continue;
         }

-        if (m_first != m_check) continue;
-        if (mi_check->fType == MFT_SEPARATOR) continue;
-
-        if (i == check)
+        if (first_item.menu != check_item.menu)
         {
-            mi_check->fType |= MFT_RADIOCHECK;
-            mi_check->fState |= MFS_CHECKED;
-            done = TRUE;
+            release_menu_ptr(check_item.menu);
+            continue;
         }
-        else
+
+        if (check_item.item->fType != MFT_SEPARATOR)
         {
-            /* MSDN is wrong, Windows does not remove MFT_RADIOCHECK */
-            mi_check->fState &= ~MFS_CHECKED;
+            if (i == check)
+            {
+                check_item.item->fType |= MFT_RADIOCHECK;
+                check_item.item->fState |= MFS_CHECKED;
+                done = TRUE;
+            }
+            else
+            {
+                /* MSDN is wrong, Windows does not remove MFT_RADIOCHECK */
+                check_item.item->fState &= ~MFS_CHECKED;
+            }
         }
+
+        release_menu_ptr(check_item.menu);
     }
+    release_menu_ptr(first_item.menu);

return done;
 }
@@ -5181,27 +5325,30 @@ BOOL WINAPI CheckMenuRadioItem(HMENU hMenu,
  */
 BOOL WINAPI GetMenuItemRect(HWND hwnd, HMENU hMenu, UINT uItem, RECT *rect)
 {
-     POPUPMENU *menu;
-     MENUITEM *item;
-
-     TRACE("(%p,%p,%d,%p)\n", hwnd, hMenu, uItem, rect);
+    struct menu_item_desc item_desc;

-     item = MENU_FindItem (&hMenu, &uItem, MF_BYPOSITION);
-     if ((rect == NULL) || (item == NULL))
-         return FALSE;
+    TRACE("(%p,%p,%d,%p)\n", hwnd, hMenu, uItem, rect);

-     menu = MENU_GetMenu(hMenu);
-     if (!menu) return FALSE;
+    if (!rect)
+        return FALSE;

-     if (!hwnd) hwnd = menu->hWnd;
-     if (!hwnd) return FALSE;
+    item_desc.id = uItem;
+    if (!MENU_FindItem(hMenu, MF_BYPOSITION, &item_desc))
+        return FALSE;

-     *rect = item->rect;
+    if (!hwnd) hwnd = item_desc.menu->hWnd;
+    if (!hwnd)
+    {
+        release_menu_ptr(item_desc.menu);
+        return FALSE;
+    }

-     OffsetRect(rect, menu->items_rect.left, menu->items_rect.top);
-     MapWindowPoints(hwnd, 0, (POINT *)rect, 2);
+    *rect = item_desc.item->rect;
+    OffsetRect(rect, item_desc.menu->items_rect.left, item_desc.menu->items_rect.top);
+    release_menu_ptr(item_desc.menu);

-     return TRUE;
+    MapWindowPoints(hwnd, 0, (POINT *)rect, 2);
+    return TRUE;
 }

/**********************************************************************
@@ -5415,16 +5562,19 @@ static BOOL translate_accelerator( HWND hWnd, UINT message, WPARAM wParam, LPARA
     {
         HMENU hMenu, hSubMenu, hSysMenu;
         UINT uSysStat = (UINT)-1, uStat = (UINT)-1, nPos;
+        struct menu_item_desc item_desc;

hMenu = (GetWindowLongW( hWnd, GWL_STYLE ) & WS_CHILD) ? 0 : GetMenu(hWnd);
         hSysMenu = get_win_sys_menu( hWnd );

/* find menu item and ask application to initialize it */
         /* 1. in the system menu */
-        hSubMenu = hSysMenu;
-        nPos = cmd;
-        if(MENU_FindItem(&hSubMenu, &nPos, MF_BYCOMMAND))
+        item_desc.id = cmd;
+        if (MENU_FindItem(hSysMenu, MF_BYCOMMAND, &item_desc))
         {
+            hSubMenu = item_desc.menu->obj.handle;
+            release_menu_ptr(item_desc.menu);
+
             if (GetCapture())
                 mesg = 2;
             if (!IsWindowEnabled(hWnd))
@@ -5443,10 +5593,12 @@ static BOOL translate_accelerator( HWND hWnd, UINT message, WPARAM wParam, LPARA
         }
         else /* 2. in the window's menu */
         {
-            hSubMenu = hMenu;
-            nPos = cmd;
-            if(MENU_FindItem(&hSubMenu, &nPos, MF_BYCOMMAND))
+            item_desc.id = cmd;
+            if (MENU_FindItem(hMenu, MF_BYCOMMAND, &item_desc))
             {
+                hSubMenu = item_desc.menu->obj.handle;
+                release_menu_ptr(item_desc.menu);
+
                 if (GetCapture())
                     mesg = 2;
                 if (!IsWindowEnabled(hWnd))