Nikolay Sivov nsivov@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@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.