[v4 PATCH] user32/menu: Return locked menu data when looking for menu items.
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- v4: changed helper interface, simplified insertion point logic a bit. dlls/user32/menu.c | 556 ++++++++++++++++++++++++++++----------------- 1 file changed, 348 insertions(+), 208 deletions(-) diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c index 537c2ebe1a..98f1fce506 100644 --- a/dlls/user32/menu.c +++ b/dlls/user32/menu.c @@ -584,26 +584,25 @@ static UINT MENU_GetStartOfPrevColumn( return i; } - - -/*********************************************************************** - * MENU_FindItem - * - * 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 POPUPMENU *find_menu_item(HMENU hmenu, UINT id, UINT flags, UINT *pos) { + 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; - if (wFlags & MF_BYPOSITION) + menu = grab_menu_ptr(hmenu); + if (!menu) + return NULL; + + if (flags & MF_BYPOSITION) { - if (*nPos >= menu->nItems) return NULL; - return &menu->items[*nPos]; + if (id >= menu->nItems) + { + release_menu_ptr(menu); + return NULL; + } + + if (pos) *pos = id; + return menu; } else { @@ -612,32 +611,36 @@ 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) + POPUPMENU *submenu = find_menu_item(item->hSubMenu, id, flags, pos); + + if (submenu) + { + release_menu_ptr(menu); + return submenu; + } + else if (item->wID == id) { /* fallback to this item if nothing else found */ fallback_pos = i; - fallback = item; } } - else if (item->wID == *nPos) + else if (item->wID == id) { - *nPos = i; - return item; + if (pos) *pos = i; + return menu; } } } - if (fallback) - *nPos = fallback_pos; + if (fallback_pos != ~0u) + *pos = fallback_pos; + else + { + release_menu_ptr(menu); + menu = NULL; + } - return fallback; + return menu; } /*********************************************************************** @@ -2131,30 +2134,22 @@ static void MENU_MoveSelection( HWND hwndOwner, HMENU hmenu, INT offset ) /********************************************************************** - * MENU_InsertItem + * insert_menu_item * * Insert (allocate) a new item into a menu. */ -static MENUITEM *MENU_InsertItem( HMENU hMenu, UINT pos, UINT flags ) +static POPUPMENU *insert_menu_item(HMENU hMenu, UINT id, UINT flags, UINT *ret_pos) { MENUITEM *newItems; POPUPMENU *menu; - - if (!(menu = MENU_GetMenu(hMenu))) - return NULL; + UINT pos = id; /* Find where to insert new item */ - - if (flags & MF_BYPOSITION) { - if (pos > menu->nItems) - pos = menu->nItems; - } else { - if (!MENU_FindItem( &hMenu, &pos, flags )) - pos = menu->nItems; - else { - if (!(menu = MENU_GetMenu( hMenu ))) - return NULL; - } + if (!(menu = find_menu_item(hMenu, id, flags, &pos))) + { + if (!(menu = grab_menu_ptr(hMenu))) + return NULL; + pos = menu->nItems; } /* Make sure that MDI system buttons stay on the right side. @@ -2172,6 +2167,7 @@ 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; } @@ -2187,7 +2183,9 @@ 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]; + + *ret_pos = pos; + return menu; } @@ -3706,13 +3704,19 @@ BOOL WINAPI ChangeMenuW( HMENU hMenu, UINT pos, LPCWSTR data, */ DWORD WINAPI CheckMenuItem( HMENU hMenu, UINT id, UINT flags ) { + POPUPMENU *menu; MENUITEM *item; DWORD ret; + UINT pos; + + if (!(menu = find_menu_item(hMenu, id, flags, &pos))) + return -1; + item = &menu->items[pos]; - 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; + release_menu_ptr(menu); return ret; } @@ -3720,42 +3724,48 @@ DWORD WINAPI CheckMenuItem( HMENU hMenu, UINT id, UINT flags ) /********************************************************************** * EnableMenuItem (USER32.@) */ -BOOL WINAPI EnableMenuItem( HMENU hMenu, UINT wItemID, UINT wFlags ) +BOOL WINAPI EnableMenuItem( HMENU hMenu, UINT id, UINT wFlags ) { - UINT oldflags; - MENUITEM *item; + UINT oldflags, pos; POPUPMENU *menu; + MENUITEM *item; - TRACE("(%p, %04x, %04x) !\n", hMenu, wItemID, wFlags); + TRACE("(%p, %04x, %04x)\n", hMenu, id, wFlags); /* Get the Popupmenu to access the owner menu */ - if (!(menu = MENU_GetMenu(hMenu))) - return (UINT)-1; - - if (!(item = MENU_FindItem( &hMenu, &wItemID, wFlags ))) + if (!(menu = find_menu_item(hMenu, id, wFlags, &pos))) return (UINT)-1; + item = &menu->items[pos]; oldflags = item->fState & (MF_GRAYED | MF_DISABLED); 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->wID == SC_CLOSE) && (oldflags != wFlags)) { - if (menu->hSysMenuOwner != 0) + if (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(menu->hSysMenuOwner); + release_menu_ptr(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(menu); return oldflags; } @@ -3770,21 +3780,37 @@ 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 */ -) { +) +{ + POPUPMENU *menu; MENUITEM *item; + UINT pos; + 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 ))) { + + if (!(menu = find_menu_item(hMenu, wItemID, wFlags, &pos))) + { 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; + item = &menu->items[pos]; + + if (!item->text) + ret = 0; + else if (!str || !nMaxSiz) + ret = WideCharToMultiByte( CP_ACP, 0, item->text, -1, NULL, 0, NULL, NULL ); + else + { + if (!WideCharToMultiByte( CP_ACP, 0, item->text, -1, str, nMaxSiz, NULL, NULL )) + str[nMaxSiz-1] = 0; + ret = strlen(str); + } + release_menu_ptr(menu); + TRACE("returning %s\n", debugstr_a(str)); - return strlen(str); + return ret; } @@ -3794,22 +3820,37 @@ INT WINAPI GetMenuStringA( INT WINAPI GetMenuStringW( HMENU hMenu, UINT wItemID, LPWSTR str, INT nMaxSiz, UINT wFlags ) { + POPUPMENU *menu; MENUITEM *item; + UINT pos; + 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 ))) { + + if (!(menu = find_menu_item(hMenu, wItemID, wFlags, &pos))) + { SetLastError( ERROR_MENU_ITEM_NOT_FOUND); return 0; } - if (!str || !nMaxSiz) return item->text ? strlenW(item->text) : 0; - if( !(item->text)) { + item = &menu->items[pos]; + + if (!str || !nMaxSiz) + ret = item->text ? strlenW(item->text) : 0; + else if (!item->text) + { str[0] = 0; - return 0; + ret = 0; + } + else + { + lstrcpynW( str, item->text, nMaxSiz ); + ret = strlenW(str); } - lstrcpynW( str, item->text, nMaxSiz ); + release_menu_ptr(menu); + TRACE("returning %s\n", debugstr_w(str)); - return strlenW(str); + return ret; } @@ -3819,13 +3860,19 @@ INT WINAPI GetMenuStringW( HMENU hMenu, UINT wItemID, BOOL WINAPI HiliteMenuItem( HWND hWnd, HMENU hMenu, UINT wItemID, UINT wHilite ) { - LPPOPUPMENU menu; + POPUPMENU *menu; + UINT pos; + 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 ); + + if (!(menu = find_menu_item(hMenu, wItemID, wHilite, &pos))) return FALSE; + + if (menu->FocusedItem != pos) + { + MENU_HideSubPopups( hWnd, menu->obj.handle, FALSE, 0 ); + MENU_SelectItem( hWnd, menu->obj.handle, pos, TRUE, 0 ); + } + release_menu_ptr(menu); return TRUE; } @@ -3835,23 +3882,35 @@ BOOL WINAPI HiliteMenuItem( HWND hWnd, HMENU hMenu, UINT wItemID, */ UINT WINAPI GetMenuState( HMENU hMenu, UINT wItemID, UINT wFlags ) { + POPUPMENU *menu; + UINT state, pos; MENUITEM *item; + TRACE("(menu=%p, id=%04x, flags=%04x);\n", hMenu, wItemID, wFlags); - if (!(item = MENU_FindItem( &hMenu, &wItemID, wFlags ))) return -1; + + if (!(menu = find_menu_item(hMenu, wItemID, wFlags, &pos))) + return -1; + + item = &menu->items[pos]; debug_print_menuitem (" item: ", item, ""); if (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->hSubMenu); + if (submenu) + state = (submenu->nItems << 8) | ((item->fState | 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->fType | item->fState); } + release_menu_ptr(menu); + return state; } @@ -3877,12 +3936,15 @@ INT WINAPI GetMenuItemCount( HMENU hMenu ) */ UINT WINAPI GetMenuItemID( HMENU hMenu, INT nPos ) { - MENUITEM * lpmi; + POPUPMENU *menu; + UINT id, pos; - if (!(lpmi = MENU_FindItem(&hMenu,(UINT*)&nPos,MF_BYPOSITION))) return -1; - if (lpmi->fType & MF_POPUP) return -1; - return lpmi->wID; + if (!(menu = find_menu_item(hMenu, nPos, MF_BYPOSITION, &pos))) + return -1; + id = menu->items[pos].fType & MF_POPUP ? -1 : menu->items[pos].wID; + release_menu_ptr(menu); + return id; } @@ -3935,8 +3997,11 @@ 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; MENUITEMINFOW mii; + POPUPMENU *menu; + MENUITEM *item; + UINT newpos; + BOOL ret; if (IS_STRING_ITEM(flags) && str) TRACE("hMenu %p, pos %d, flags %08x, id %04lx, str %s\n", @@ -3944,16 +4009,20 @@ 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; + if (!(menu = insert_menu_item(hMenu, pos, flags, &newpos))) + return FALSE; + MENU_mnu2mnuii( flags, id, str, &mii); - if (!(SetMenuItemInfo_common( item, &mii, TRUE))) - { + + item = &menu->items[newpos]; + ret = SetMenuItemInfo_common( item, &mii, TRUE); + if (ret) + item->hCheckBit = item->hUnCheckBit = 0; + else RemoveMenu( hMenu, pos, flags ); - return FALSE; - } + release_menu_ptr(menu); - item->hCheckBit = item->hUnCheckBit = 0; - return TRUE; + return ret; } @@ -4006,16 +4075,16 @@ BOOL WINAPI AppendMenuW( HMENU hMenu, UINT flags, */ BOOL WINAPI RemoveMenu( HMENU hMenu, UINT nPos, UINT wFlags ) { - LPPOPUPMENU menu; - MENUITEM *item; + POPUPMENU *menu; + UINT pos; 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 */ + if (!(menu = find_menu_item(hMenu, nPos, wFlags, &pos))) + return FALSE; - MENU_FreeItemData( item ); + /* Remove item */ + MENU_FreeItemData( &menu->items[pos] ); if (--menu->nItems == 0) { @@ -4024,17 +4093,19 @@ BOOL WINAPI RemoveMenu( HMENU hMenu, UINT nPos, UINT wFlags ) } else { - MENUITEM *new_items; - while(nPos < menu->nItems) + MENUITEM *new_items, *item = &menu->items[pos]; + + while (nPos < menu->nItems) { *item = *(item+1); item++; nPos++; } - new_items = HeapReAlloc( GetProcessHeap(), 0, menu->items, - menu->nItems * sizeof(MENUITEM) ); + new_items = HeapReAlloc( GetProcessHeap(), 0, menu->items, menu->nItems * sizeof(MENUITEM) ); if (new_items) menu->items = new_items; } + release_menu_ptr(menu); + return TRUE; } @@ -4044,11 +4115,17 @@ 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 ); + POPUPMENU *menu; + UINT pos; + + if (!(menu = find_menu_item(hMenu, nPos, wFlags, &pos))) + return FALSE; + + if (menu->items[pos].fType & MF_POPUP) + DestroyMenu(menu->items[pos].hSubMenu); + + RemoveMenu(menu->obj.handle, pos, wFlags | MF_BYPOSITION); + release_menu_ptr(menu); return TRUE; } @@ -4059,23 +4136,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; MENUITEMINFOW mii; + POPUPMENU *menu; + UINT item_pos; + 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 ))) + if (!(menu = find_menu_item(hMenu, pos, flags, &item_pos))) { /* 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 */ + menu->Height = 0; /* force size recalculate */ MENU_mnu2mnuii( flags, id, str, &mii); - return SetMenuItemInfo_common( item, &mii, TRUE); + ret = SetMenuItemInfo_common(&menu->items[item_pos], &mii, TRUE); + release_menu_ptr(menu); + return ret; } @@ -4134,20 +4215,26 @@ DWORD WINAPI GetMenuCheckMarkDimensions(void) BOOL WINAPI SetMenuItemBitmaps( HMENU hMenu, UINT nPos, UINT wFlags, HBITMAP hNewUnCheck, HBITMAP hNewCheck) { + POPUPMENU *menu; MENUITEM *item; + UINT pos; - if (!(item = MENU_FindItem( &hMenu, &nPos, wFlags ))) return FALSE; + if (!(menu = find_menu_item(hMenu, nPos, wFlags, &pos))) + return FALSE; + item = &menu->items[pos]; if (!hNewCheck && !hNewUnCheck) { - item->fState &= ~MF_USECHECKBITMAPS; + item->fState &= ~MF_USECHECKBITMAPS; } else /* Install new bitmaps */ { - item->hCheckBit = hNewCheck; - item->hUnCheckBit = hNewUnCheck; - item->fState |= MF_USECHECKBITMAPS; + item->hCheckBit = hNewCheck; + item->hUnCheckBit = hNewUnCheck; + item->fState |= MF_USECHECKBITMAPS; } + release_menu_ptr(menu); + return TRUE; } @@ -4418,11 +4505,20 @@ BOOL WINAPI SetMenu( HWND hWnd, HMENU hMenu ) */ HMENU WINAPI GetSubMenu( HMENU hMenu, INT nPos ) { - MENUITEM * lpmi; + POPUPMENU *menu; + HMENU submenu; + UINT pos; + + if (!(menu = find_menu_item(hMenu, nPos, MF_BYPOSITION, &pos))) + return 0; - if (!(lpmi = MENU_FindItem(&hMenu,(UINT*)&nPos,MF_BYPOSITION))) return 0; - if (!(lpmi->fType & MF_POPUP)) return 0; - return lpmi->hSubMenu; + if (menu->items[pos].fType & MF_POPUP) + submenu = menu->items[pos].hSubMenu; + else + submenu = 0; + + release_menu_ptr(menu); + return submenu; } @@ -4631,31 +4727,39 @@ BOOL WINAPI IsMenu(HMENU hmenu) * GetMenuItemInfo_common */ -static BOOL GetMenuItemInfo_common ( HMENU hmenu, UINT item, BOOL bypos, +static BOOL GetMenuItemInfo_common ( HMENU hmenu, UINT id, BOOL bypos, LPMENUITEMINFOW lpmii, BOOL unicode) { - MENUITEM *menu = MENU_FindItem (&hmenu, &item, bypos ? MF_BYPOSITION : 0); + POPUPMENU *menu; + MENUITEM *item; + UINT pos; - debug_print_menuitem("GetMenuItemInfo_common: ", menu, ""); + menu = find_menu_item(hmenu, id, bypos ? MF_BYPOSITION : 0, &pos); - if (!menu) { + item = menu ? &menu->items[pos] : NULL; + + debug_print_menuitem("GetMenuItemInfo_common: ", item, ""); + + if (!menu) + { 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(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->fType & MENUITEMINFO_TYPE_MASK; + if (item->hbmpItem && !IS_MAGIC_BITMAP(item->hbmpItem)) lpmii->fType |= MFT_BITMAP; - lpmii->hbmpItem = menu->hbmpItem; /* not on Win9x/ME */ + lpmii->hbmpItem = item->hbmpItem; /* not on Win9x/ME */ if( lpmii->fType & MFT_BITMAP) { - lpmii->dwTypeData = (LPWSTR) menu->hbmpItem; + lpmii->dwTypeData = (LPWSTR) item->hbmpItem; lpmii->cch = 0; } else if( lpmii->fType & (MFT_OWNERDRAW | MFT_SEPARATOR)) { /* this does not happen on Win9x/ME */ @@ -4666,7 +4770,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->text) { if(lpmii->dwTypeData && lpmii->cch) { if( unicode) *((WCHAR *)lpmii->dwTypeData) = 0; @@ -4678,16 +4782,16 @@ static BOOL GetMenuItemInfo_common ( HMENU hmenu, UINT item, BOOL bypos, int len; if (unicode) { - len = strlenW(menu->text); + len = strlenW(item->text); if(lpmii->dwTypeData && lpmii->cch) - lstrcpynW(lpmii->dwTypeData, menu->text, lpmii->cch); + lstrcpynW(lpmii->dwTypeData, item->text, lpmii->cch); } else { - len = WideCharToMultiByte( CP_ACP, 0, menu->text, -1, NULL, + len = WideCharToMultiByte( CP_ACP, 0, 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->text, -1, (LPSTR)lpmii->dwTypeData, lpmii->cch, NULL, NULL )) ((LPSTR)lpmii->dwTypeData)[lpmii->cch - 1] = 0; } @@ -4706,19 +4810,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->fType & MENUITEMINFO_TYPE_MASK; if (lpmii->fMask & MIIM_BITMAP) - lpmii->hbmpItem = menu->hbmpItem; + lpmii->hbmpItem = item->hbmpItem; if (lpmii->fMask & MIIM_STATE) - lpmii->fState = menu->fState & MENUITEMINFO_STATE_MASK; + lpmii->fState = item->fState & MENUITEMINFO_STATE_MASK; if (lpmii->fMask & MIIM_ID) - lpmii->wID = menu->wID; + lpmii->wID = item->wID; if (lpmii->fMask & MIIM_SUBMENU) - lpmii->hSubMenu = menu->hSubMenu; + lpmii->hSubMenu = item->hSubMenu; else { /* hSubMenu is always cleared * (not on Win9x/ME ) */ @@ -4726,13 +4830,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->hCheckBit; + lpmii->hbmpUnchecked = item->hUnCheckBit; } if (lpmii->fMask & MIIM_DATA) - lpmii->dwItemData = menu->dwItemData; + lpmii->dwItemData = item->dwItemData; - return TRUE; + release_menu_ptr(menu); + return TRUE; } /********************************************************************** @@ -4949,20 +5054,24 @@ static BOOL MENU_NormalizeMenuItemInfoStruct( const MENUITEMINFOW *pmii_in, BOOL WINAPI SetMenuItemInfoA(HMENU hmenu, UINT item, BOOL bypos, const MENUITEMINFOA *lpmii) { - MENUITEM *menuitem; MENUITEMINFOW mii; + POPUPMENU *menu; + UINT pos; + 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 ))) + if (!(menu = find_menu_item(hmenu, item, bypos ? MF_BYPOSITION : 0, &pos))) { /* 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(&menu->items[pos], &mii, FALSE); + release_menu_ptr(menu); + return ret; } /********************************************************************** @@ -4971,19 +5080,25 @@ BOOL WINAPI SetMenuItemInfoA(HMENU hmenu, UINT item, BOOL bypos, BOOL WINAPI SetMenuItemInfoW(HMENU hmenu, UINT item, BOOL bypos, const MENUITEMINFOW *lpmii) { - MENUITEM *menuitem; MENUITEMINFOW mii; + POPUPMENU *menu; + BOOL ret; + UINT pos; 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 ))) + + if (!(menu = find_menu_item(hmenu, item, bypos ? MF_BYPOSITION : 0, &pos))) { /* 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(&menu->items[pos], &mii, TRUE); + release_menu_ptr(menu); + return ret; } static BOOL set_menu_default_item(POPUPMENU *menu, UINT uItem, UINT bypos) @@ -5090,15 +5205,21 @@ UINT WINAPI GetMenuDefaultItem(HMENU hmenu, UINT bypos, UINT flags) BOOL WINAPI InsertMenuItemA(HMENU hMenu, UINT uItem, BOOL bypos, const MENUITEMINFOA *lpmii) { - MENUITEM *item; MENUITEMINFOW mii; + POPUPMENU *menu; + UINT pos; + 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); + if (!(menu = insert_menu_item(hMenu, uItem, bypos ? MF_BYPOSITION : 0, &pos))) + return FALSE; + + ret = SetMenuItemInfo_common(&menu->items[pos], &mii, FALSE); + release_menu_ptr(menu); + return ret; } @@ -5108,64 +5229,77 @@ BOOL WINAPI InsertMenuItemA(HMENU hMenu, UINT uItem, BOOL bypos, BOOL WINAPI InsertMenuItemW(HMENU hMenu, UINT uItem, BOOL bypos, const MENUITEMINFOW *lpmii) { - MENUITEM *item; MENUITEMINFOW mii; + POPUPMENU *menu; + UINT pos; + 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); + if (!(menu = insert_menu_item(hMenu, uItem, bypos ? MF_BYPOSITION : 0, &pos))) + return FALSE; + + ret = SetMenuItemInfo_common(&menu->items[pos], &mii, TRUE); + release_menu_ptr(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) { + POPUPMENU *first_menu = NULL, *check_menu = NULL; + UINT i, first_pos, check_pos; 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; + MENUITEM *item; - if (!mi_first) + if (!first_menu) { - m_first = hMenu; - mi_first = MENU_FindItem(&m_first, &pos, bypos); - if (!mi_first) continue; - mi_check = mi_first; - m_check = m_first; + if (!(first_menu = find_menu_item(hMenu, i, flags, &first_pos))) + continue; + + check_pos = first_pos; + check_menu = grab_menu_ptr(first_menu->obj.handle); } else { - m_check = hMenu; - mi_check = MENU_FindItem(&m_check, &pos, bypos); - if (!mi_check) continue; + if (!(check_menu = find_menu_item(hMenu, i, flags, &check_pos))) + continue; } - if (m_first != m_check) continue; - if (mi_check->fType == MFT_SEPARATOR) continue; - - if (i == check) + if (first_menu != check_menu) { - mi_check->fType |= MFT_RADIOCHECK; - mi_check->fState |= MFS_CHECKED; - done = TRUE; + release_menu_ptr(check_menu); + continue; } - else + + item = &check_menu->items[check_pos]; + if (item->fType != MFT_SEPARATOR) { - /* MSDN is wrong, Windows does not remove MFT_RADIOCHECK */ - mi_check->fState &= ~MFS_CHECKED; + if (i == check) + { + item->fType |= MFT_RADIOCHECK; + item->fState |= MFS_CHECKED; + done = TRUE; + } + else + { + /* MSDN is wrong, Windows does not remove MFT_RADIOCHECK */ + item->fState &= ~MFS_CHECKED; + } } + + release_menu_ptr(check_menu); } + release_menu_ptr(first_menu); return done; } @@ -5181,27 +5315,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); + POPUPMENU *menu; + UINT pos; - 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; + if (!(menu = find_menu_item(hMenu, uItem, MF_BYPOSITION, &pos))) + return FALSE; - *rect = item->rect; + if (!hwnd) hwnd = menu->hWnd; + if (!hwnd) + { + release_menu_ptr(menu); + return FALSE; + } - OffsetRect(rect, menu->items_rect.left, menu->items_rect.top); - MapWindowPoints(hwnd, 0, (POINT *)rect, 2); + *rect = menu->items[pos].rect; + OffsetRect(rect, menu->items_rect.left, menu->items_rect.top); + release_menu_ptr(menu); - return TRUE; + MapWindowPoints(hwnd, 0, (POINT *)rect, 2); + return TRUE; } /********************************************************************** @@ -5415,16 +5552,18 @@ static BOOL translate_accelerator( HWND hWnd, UINT message, WPARAM wParam, LPARA { HMENU hMenu, hSubMenu, hSysMenu; UINT uSysStat = (UINT)-1, uStat = (UINT)-1, nPos; + POPUPMENU *menu; 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)) + if ((menu = find_menu_item(hSysMenu, cmd, MF_BYCOMMAND, NULL))) { + hSubMenu = menu->obj.handle; + release_menu_ptr(menu); + if (GetCapture()) mesg = 2; if (!IsWindowEnabled(hWnd)) @@ -5443,10 +5582,11 @@ 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)) + if ((menu = find_menu_item(hMenu, cmd, MF_BYCOMMAND, NULL))) { + hSubMenu = menu->obj.handle; + release_menu_ptr(menu); + if (GetCapture()) mesg = 2; if (!IsWindowEnabled(hWnd)) -- 2.17.0
On Wed, May 09, 2018 at 12:04:28PM +0300, Nikolay Sivov wrote:
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> ---
v4: changed helper interface, simplified insertion point logic a bit.
dlls/user32/menu.c | 556 ++++++++++++++++++++++++++++----------------- 1 file changed, 348 insertions(+), 208 deletions(-)
diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c index 537c2ebe1a..98f1fce506 100644 --- a/dlls/user32/menu.c +++ b/dlls/user32/menu.c @@ -584,26 +584,25 @@ static UINT MENU_GetStartOfPrevColumn( return i; }
- - -/*********************************************************************** - * MENU_FindItem - * - * 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 POPUPMENU *find_menu_item(HMENU hmenu, UINT id, UINT flags, UINT *pos)
I think this is more natural than the previous version.
BOOL WINAPI RemoveMenu( HMENU hMenu, UINT nPos, UINT wFlags ) { - LPPOPUPMENU menu; - MENUITEM *item; + POPUPMENU *menu; + UINT pos;
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 */ + if (!(menu = find_menu_item(hMenu, nPos, wFlags, &pos))) + return FALSE;
- MENU_FreeItemData( item ); + /* Remove item */ + MENU_FreeItemData( &menu->items[pos] );
if (--menu->nItems == 0) { @@ -4024,17 +4093,19 @@ BOOL WINAPI RemoveMenu( HMENU hMenu, UINT nPos, UINT wFlags ) } else { - MENUITEM *new_items; - while(nPos < menu->nItems) + MENUITEM *new_items, *item = &menu->items[pos]; + + while (nPos < menu->nItems) { *item = *(item+1); item++; nPos++; }
These should be pos not nPos. I'd suggest changing the function's parameter name from nPos to id.
- new_items = HeapReAlloc( GetProcessHeap(), 0, menu->items, - menu->nItems * sizeof(MENUITEM) ); + new_items = HeapReAlloc( GetProcessHeap(), 0, menu->items, menu->nItems * sizeof(MENUITEM) ); if (new_items) menu->items = new_items; } + release_menu_ptr(menu); + return TRUE; }
@@ -4044,11 +4115,17 @@ BOOL WINAPI RemoveMenu( HMENU hMenu, UINT nPos, UINT wFlags ) */ BOOL WINAPI DeleteMenu( HMENU hMenu, UINT nPos, UINT wFlags ) {
Probably change nPos to id here as well. Huw.
participants (2)
-
Huw Davies -
Nikolay Sivov