[PATCH] user32: Don't destroy in-use menus

[Andrew Eikum]

On Thu, May 01, 2014 at 04:38:44PM -0500, Ken Thomases wrote:
> On May 1, 2014, at 2:19 PM, Andrew Eikum wrote:
> 
> 
> > +    /* two different kinds of "reference" counting. in_use is for internal
> > +     * refcounting for e.g. submenus. user_destroyed indicates whether
> > +     * DestroyMenu has been invoked on a handle returned by CreateMenu. */
> > +    BOOL        in_use, user_destroyed;
> 
> First, since in_use is a count, it probably shouldn't be a BOOL.
> 
> Second, I haven't reviewed the menu code thoroughly, but I think that it would be better to conceptually separate the handle from the internal structure.  Internal references to submenus and the like should be by pointers to the structure.  The handle should only be used at the interface with external clients.
> 
> Then, there should be only one reference count.  It's the count of users of the structure.  The handle represents one reference.  Each place where a pointer to the structure is stored is another reference.
> 

I see what you mean. I had thought the submenu handle was used in the
TrackPopupMenu test directly, but it actually gets the parent menu
handle. Meaning we don't need to keep the submenu handle valid, even
though GetMenuItemInfo actually returns the destroyed submenu handle.

> DestroyMenu() should really destroy the handle (and thus decrement the reference count), not just mark it as "destroyed".
> 

I'll try this along with your handle/struct separation suggestion.
Thanks for reviewing.

Andrew