On Mon, Apr 30, 2018 at 03:11:25PM +0300, Nikolay Sivov wrote:
+struct menu_item_desc +{ + MENUITEM *item; + POPUPMENU *menu; + UINT pos; +};
/*********************************************************************** * 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 BOOL MENU_FindItem(HMENU hmenu, UINT id, UINT flags, struct menu_item_desc *desc)
I'd have probably used the chance to rename this as find_item() ;-) I also wondered about having this just return the menu and pos (in which case it's find_item_pos()) and then either letting the callers index directly into to the returned menu's items array, or adding a get_item() that would do that. I suppose though there are enough callers that want the item, that this combined approach is a good compromise.
{ + 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) + memset(desc, 0, sizeof(*desc)); + + menu = grab_menu_ptr(hmenu); + if (!menu) + return FALSE; + + if (flags & MF_BYPOSITION) { - if (*nPos >= menu->nItems) return NULL; - return &menu->items[*nPos]; + if (id >= menu->nItems) + { + release_menu_ptr(menu); + return FALSE; + } + + desc->menu = menu; + desc->item = &menu->items[id]; + desc->pos = id; + return TRUE; } else { @@ -612,32 +625,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, id, flags, desc)) + { + release_menu_ptr(menu); + return TRUE; + } + 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; + desc->menu = menu; + desc->item = item; + desc->pos = i; + return TRUE; } } }
- if (fallback) - *nPos = fallback_pos; + if (fallback_pos != ~0u) + { + desc->menu = menu; + desc->item = &menu->items[fallback_pos]; + desc->pos = fallback_pos; + } + else + release_menu_ptr(menu);
- return fallback; + return fallback_pos != ~0u; }
/*********************************************************************** @@ -2135,28 +2153,34 @@ 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 id, UINT flags, struct menu_item_desc *desc) { + UINT pos = id; MENUITEM *newItems; POPUPMENU *menu;
- if (!(menu = MENU_GetMenu(hMenu))) - return NULL; + memset(desc, 0, sizeof(*desc));
/* Find where to insert new item */ - if (flags & MF_BYPOSITION) { - if (pos > menu->nItems) + menu = grab_menu_ptr(hMenu); + if (id > menu->nItems) pos = menu->nItems; } else { - if (!MENU_FindItem( &hMenu, &pos, flags )) + struct menu_item_desc item_desc; + + if (!MENU_FindItem(hMenu, id, flags, &item_desc)) + { + menu = grab_menu_ptr(hMenu); pos = menu->nItems; - else { - if (!(menu = MENU_GetMenu( hMenu ))) - return NULL; } + else + menu = item_desc.menu;
Shouldn't you use the returned item_desc.pos here? Also, to simplify things, couldn't the MF_BYPOSITION case just call MENU_FindItem() too? Huw.