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

[Alexandre Julliard]

Nikolay Sivov <nsivov at codeweavers.com> writes:

> 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.

I'm not sure we want a back link. You could potentially return a struct
containing both MENUITEM and POPUPMENU. I also feel you'd want to keep a
helper that takes an HMENU as input, to avoid adding complexity to most
callers.

-- 
Alexandre Julliard
julliard at winehq.org