Re: Rein Klazes : user32: Separate menu bitmaps and strings.

[paul]

Alexandre Julliard wrote:
> Module: wine
> Branch: refs/heads/master
> Commit: b34868dffa0df144ea9d6e1586a67cff70ae9c4e
> URL:    http://source.winehq.org/git/?p=wine.git;a=commit;h=b34868dffa0df144ea9d6e15...
>
> Author: Rein Klazes <wijn(a)wanadoo.nl>
> Date:   Tue Jan  3 13:23:28 2006 +0100
>
> user32: Separate menu bitmaps and strings.
> Store bitmaps and bitmaps always in separate fields in the internal
> menu structure.  Get rid of a lot of assumptions in the code that the
> menu can have strings only when it is not some other type and that
> bitmaps come in two flavors.
> Add a lot of conformance tests, including some submitted by Jason
> Edmeades.
>
> ---
>
> dlls/user/menu.c       |  317 ++++++++++++++++----------
>  dlls/user/tests/menu.c |  593 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 785 insertions(+), 125 deletions(-)

Regression see http://bugs.winehq.org/show_bug.cgi?id=4250

The set last error function added to GetMenuStringA and GetMenuStringW 
removed this check which seems to be still needed.
if (!IS_STRING_ITEM(item->fType)) return 0;

I have tried several times to make a patch of the changes required but I 
get a lot of white space changes.

Paul R.

Rein Klazes : user32: Separate menu bitmaps and strings.

[Rein Klazes]

On Fri, 06 Jan 2006 22:41:00 -0500, you wrote:
> The set last error function added to GetMenuStringA and GetMenuStringW 
> removed this check which seems to be still needed.
> if (!IS_STRING_ITEM(item->fType)) return 0;
>
> I have tried several times to make a patch of the changes required but I 
> get a lot of white space changes.

Hi Paul,

Here is a better fix (because it does not make any regression test
fail). Let me know if it works for you too.

Changelog:
dlls/user		: menu.c
dlls/user/tests	: menu.c
Prevent a crash in GetMenuStringA caused by calling strlenW on a NULL
pointer, with a test.

Rein.

Rein Klazes : user32: Separate menu bitmaps and strings.

[Paul Romanyszyn]

Rein Klazes wrote:
> On Fri, 06 Jan 2006 22:41:00 -0500, you wrote:
> > The set last error function added to GetMenuStringA and GetMenuStringW 
> > removed this check which seems to be still needed.
> > if (!IS_STRING_ITEM(item->fType)) return 0;
> >
> > I have tried several times to make a patch of the changes required but I 
> > get a lot of white space changes.
>
> Hi Paul,
>
> Here is a better fix (because it does not make any regression test
> fail). Let me know if it works for you too.
>
> Changelog:
> dlls/user		: menu.c
> dlls/user/tests	: menu.c
> Prevent a crash in GetMenuStringA caused by calling strlenW on a NULL
> pointer, with a test.
>
> Rein.

That also fixed bug 4250
> ------------------------------------------------------------------------
>
> --- wine/dlls/user/menu.c	2006-01-04 19:02:33.000000000 +0100
> +++ mywine/dlls/user/menu.c	2006-01-07 11:45:52.000000000 +0100
> @@ -3295,7 +3295,7 @@ INT WINAPI GetMenuStringA(
>         SetLastError( ERROR_MENU_ITEM_NOT_FOUND);
>         return 0;
>     }
> -    if (!str || !nMaxSiz) return strlenW(item->text);
> +    if (!str || !nMaxSiz) return item->text ? strlenW(item->text) : 0;
>     if (!WideCharToMultiByte( CP_ACP, 0, item->text, -1, str, nMaxSiz, NULL, NULL ))
>         str[nMaxSiz-1] = 0;
>     TRACE("returning '%s'\n", str );
> --- wine/dlls/user/tests/menu.c	2006-01-03 15:37:26.000000000 +0100
> +++ mywine/dlls/user/tests/menu.c	2006-01-07 11:48:21.000000000 +0100
> @@ -348,6 +348,19 @@ static void test_menu_add_string( void )
>     ok (GetMenuString( hmenu, 0, strback, 99, MF_BYPOSITION), "GetMenuString on ownerdraw entry failed\n");
>     ok (!strcmp( strback, "string2" ), "Menu text from Ansi version incorrect\n");
>
> +    /*  crashes with wine 0.9.5 */
> +    memset(&info, 0x00, sizeof(info));
> +    info.cbSize= sizeof(MENUITEMINFO); 
> +    info.fMask= MIIM_FTYPE | MIIM_STRING; /* Set OwnerDraw + typeData */
> +    info.fType= MFT_OWNERDRAW;
> +    rc = InsertMenuItem( hmenu, 0, TRUE, &info );
> +    ok (rc, "InsertMenuItem failed\n");
> +    ok (!GetMenuString( hmenu, 0, NULL, 0, MF_BYPOSITION),
> +            "GetMenuString on ownerdraw entry succeeded.\n");
> +    ok (!GetMenuStringW( hmenu, 0, NULL, 0, MF_BYPOSITION),
> +            "GetMenuStringW on ownerdraw entry succeeded.\n");
> +
> +
>     DestroyMenu( hmenu );
> }