[v2 PATCH] user32/menu: Return locked menu data when looking for menu items.
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- 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)) -- 2.17.0
On Wed, Apr 25, 2018 at 07:27:36PM +0300, Nikolay Sivov wrote:
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> ---
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) {
IMHO using the 'id' member of desc as an [in,out] param is confusing, not least because of its name. I'd suggest changing the name to 'pos' and using it as [out]-only, then adding a UINT 'id' parameter to the function. Huw.
On 04/30/2018 10:47 AM, Huw Davies wrote:
On Wed, Apr 25, 2018 at 07:27:36PM +0300, Nikolay Sivov wrote:
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> ---
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) { IMHO using the 'id' member of desc as an [in,out] param is confusing, not least because of its name. I'd suggest changing the name to 'pos' and using it as [out]-only, then adding a UINT 'id' parameter to the function.
Ok, that makes sense.
Huw.
participants (2)
-
Huw Davies -
Nikolay Sivov