[PATCH] user32/menu: Don't use menu handles when looking for menu items.

[Nikolay Sivov]

On 04/24/2018 11:56 AM, Huw Davies wrote:

> On Mon, Apr 23, 2018 at 12:31:21PM +0300, Nikolay Sivov wrote:
>> Signed-off-by: Nikolay Sivov <nsivov at codeweavers.com>
>> ---
>>   dlls/user32/menu.c | 544 ++++++++++++++++++++++++++++++---------------
>>   1 file changed, 369 insertions(+), 175 deletions(-)
>>
>> diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c
>> index 537c2ebe1a..dcec42cdf2 100644
>> --- a/dlls/user32/menu.c
>> +++ b/dlls/user32/menu.c
>> @@ -592,52 +592,56 @@ 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 MENUITEM *MENU_FindItem(POPUPMENU **menu, UINT *nPos, UINT wFlags)
>>   {
> Perhaps it would be better to introduce some new functions here - the
> [in, out] nature of the first two params is ugly.
>
> Many callers don't need the menu or the position back, so how about:
>
> MENUITEM *find_item( POPUPMENU *menu, UINT id, UINT flags )
>
> for those cases.  That could be written in terms of MENU_FindItem()
> (for now) and callers that only need the item could be moved first.

The issue is that it's recursive to traverse submenus. Not returning 
potentially new menu pointer
limits such helper to MF_BYPOSITION case. Limited like that it will 
become more get_menu_item() than
find_menu_item(). Another way, I didn't check how intrusive that would 
be, is to add back link from
MENUITEM to its POPUPMENU. Doing so will make calling site more complex, 
because you'll have to release
conditionally or have two release calls for 'menu' and MENUITEM->menu to 
cover both cases.

>
> The rest could call something like:
>
> MENUITEM *find_item_and_popup( POPUPMENU *menu, UINT id, UINT flags,
>           POPUPMENU **popup, UINT *pos )
>
> which on success would always return a properly ref counted menu in *popup
> (either the original or a submenu).
>
>
> Huw.