Re: shell32: Write-strings warning fix

[Robert Shearman]

Andrew Talbot wrote:
> Changelog:
>    shell32: Write-strings warning fix.
>
> diff -urN a/dlls/shell32/shlview.c b/dlls/shell32/shlview.c
> --- a/dlls/shell32/shlview.c	2006-05-23 17:24:50.000000000 +0100
> +++ b/dlls/shell32/shlview.c	2006-06-07 19:37:41.000000000 +0100
> @@ -779,7 +779,9 @@
> */
>
> static void ShellView_MergeViewMenu(IShellViewImpl * This, HMENU hSubMenu)
> -{	MENUITEMINFOA	mii;
> +{
> -
>
> MENUITEMINFOA	mii;
> -
>
> static CHAR mii_type_data[] = "View";
>
> TRACE("(%p)->(submenu=%p)\n",This,hSubMenu);
>
> @@ -791,7 +793,7 @@
>      mii.cbSize = sizeof(mii);
>      mii.fMask = MIIM_SUBMENU | MIIM_TYPE | MIIM_DATA;
>      mii.fType = MFT_STRING;
> -
>  mii.dwTypeData = "View";
> -
>      mii.dwTypeData = mii_type_data;
> mii.hSubMenu = LoadMenuA(shell32_hInstance, "MENU_001");
>    InsertMenuItemA(hSubMenu, FCIDM_MENU_VIEW_SEP_OPTIONS, FALSE, &mii);
>  }

Sorry, this is wrong. If InsertMenuItemA writes to the string then it 
will be corrupted and will display the wrong thing for further uses in 
the process. It is therefore better to cast it from a "const char *" 
into a "char *" or to make it a non-static array at the top of the function.

I think the first choice of keeping it a "const char *" is the best 
choice in this case.

shell32: Write-strings warning fix

[Andrew Talbot]

Robert Shearman wrote:
> Sorry, this is wrong. If InsertMenuItemA writes to the string then it
> will be corrupted and will display the wrong thing for further uses in
> the process.

A MENUITEMINFOA's dwTypeData is currently declared as writeable (LPSTR);
what it is initialised from will not affect that, but initialising it from
a non-const array does prevent a write-strings warning. So, assuming that
one eventual aim is to add -Wwrite-strings and -Wcast-qual to the make
flags, string variables that can be constified should be, and ones that
cannot (for example, because they must conform to the SDK) need to be
initialised from non-const sources, to reduce compiler noise. I consider
this variable to be constrained to remain writeable by compatibility
issues; otherwise, the best solution would be to declare it to be of type
LPCSTR.
> It is therefore better to cast it from a "const char *" 
> into a "char *"

A statement like

char *s = (char *) "string";

would raise a cast-qual warning, when that flag gets turned on.
> or to make it a non-static array at the top of the
> function.

My assumption, in declaring such a string-variable static, is that, for
functions that get called repeatedly, microseconds will be saved on the
second and each subsequent invocation, since the local variable will not
suffer repeated assignments of an invariant value. (Big deal? Maybe, but
harmless.) However, I think it is irrelevant to the issue in hand.

-- Andy.

shell32: Write-strings warning fix

[Andrew Talbot]

Andrew Talbot wrote:
> Robert Shearman wrote:
> > Sorry, this is wrong. If InsertMenuItemA writes to the string then it
> > will be corrupted and will display the wrong thing for further uses in
> > the process.
>
> A MENUITEMINFOA's dwTypeData is currently declared as writeable (LPSTR);
> what it is initialised from will not affect that.

Sorry. On reflection, I see that my thinking was faulty: I now realise that
the structure member points to the initialising or assigning string -
rather than getting a copy of the data from it - so the writeability of the
latter does matter.

-- Andy.