How to initialize LPWSTR from a const string?

Question

I am trying to initialize MENUITEMINFO for a call to InsertMenuItem. When trying to assign a const string to dwTypeData, I get an error. The code below is from MSDN samples.

I get an error for both types of assignment

mii.dwTypeData = "&Sample text";
mii.dwTypeData = L"&Sample text";

I am using Visual Studio 2019.

  MENUITEMINFO mii = { sizeof(mii) };
  mii.fMask = MIIM_STRING | MIIM_FTYPE | MIIM_ID | MIIM_STATE;
  mii.wID = idCmdFirst + IDM_DISPLAY;
  mii.fType = MFT_STRING;
  mii.dwTypeData = L"&Sample Text";
  mii.fState = MFS_ENABLED;
  if (!InsertMenuItem(hMenu, indexMenu, TRUE, &mii))
  {
    return HRESULT_FROM_WIN32(GetLastError());
  }

The error is Error (active) E0144 a value of type "const wchar_t *" cannot be used to initialize an entity of type "wchar_t *"

According Microsoft documentation, the second one should work. https://learn.microsoft.com/en-us/windows/win32/learnwin32/working-with-strings

Edit: This is not solved by I cannot initializate WCHAR because I cannot change the type as suggested in that answer.

The issue isn't the type of char, the issue is `const` and the poor Win32 API. In this case I think you'd be fine if you added a const cast. `mii.dwTypeData = const_cast(L"&Sample Text");`. — john, Aug 22 '19 at 14:43
Possible duplicate of [I cannot initializate WCHAR](https://stackoverflow.com/questions/52137990/i-cannot-initializate-wchar) — GSerg, Aug 22 '19 at 14:43
Yeah that article is wildly buggy, or at least out of date. String literals have had `const` on them for years now. — Lightness Races in Orbit, Aug 22 '19 at 14:47
GSerg: Answer in that question is asking to change the declaration which I cannot. — workvact, Aug 22 '19 at 14:54
source of this of course not vs2019 but */Zc:strictStrings* option (The */permissive-* compiler option implicitly sets this option and */permissive-* is on by default in vs2019 new project settings) — RbMm, Aug 22 '19 at 16:05
@RbMm Thanks. I pulled the source from a sample VS 2010 project and assumed some change to the defaults in VS 2019. — workvact, Aug 22 '19 at 16:52

Anders · Accepted Answer · 2019-08-23T10:53:03.423

13

Some Windows structs are used to both "Get and Set" and string members in these structs point to mutable strings. This is in direct conflict with the compiler/linker settings that store strings literals in read-only memory.

It is theoretically unsafe to use a string literal with the setter function because it might write to the string (and then restore it back to its original content).

The only known place where this happens is the command line parameter in CreateProcessW.

In all other places you can probably just cast away const:

MENUITEMINFO mii = { sizeof(mii) };
mii.dwTypeData = const_cast<LPTSTR>(TEXT("&Sample Text"));

edited Aug 23 '19 at 10:53

answered Aug 22 '19 at 14:43

Anders

97,548
12
110
164

2

Modifying a string literal has undefined behaviour regardless of which molecules of your computer are used to store it – Lightness Races in Orbit Aug 22 '19 at 14:47
1

@LightnessRacesinOrbit Yes but the Visual C++ compiler used to default to putting string literals in read/write memory so people just ignored the issue. – Anders Aug 22 '19 at 15:14
1

simply all this api was designed much earlier /Zc:strictStrings option. by default string literals was in read only memory – RbMm Aug 22 '19 at 15:17
@Anders Indeed, including the people who wrote the MSDN article. What I'm saying is that the warning/explanation in this answer needs to be stronger IMO. Currently it sounds like flipping some linker settings will solve the problem, but it won't: the string literals have the wrong types. – Lightness Races in Orbit Aug 22 '19 at 15:19
1

unfortunately old flaw of windows api. especially native api. for example big family of functions which got `POBJECT_ATTRIBUTES` as *in* parameter. despite it only in parameter and can be in const memory - it not declared as const pointer. but say `LsaOpenPolicy` (yet one known exception) by some reason require not const `OBJECT_ATTRIBUTES` pointer – RbMm Aug 22 '19 at 15:34
@lig: No one is modifying the string literal, though. This is an artifact of the fact, that you cannot transitively convey `const`-ness of pointees in C or C++. Casting away `const` and passing a string literal is safe here. I'm not sure what warning should be more explicit or emphasized. – IInspectable Aug 22 '19 at 21:08
@IInspectable _"No-one is modifying the string literal"_ By stripping away the `const` you lose that guarantee (though I do agree that, in practice, in this particular example, and as explained by Anders, you're probably going to be alright) and the potential issues with this in general are only really touched on in this answer with respect to "compiler/linker settings that store strings in read-only memory", which completely sidesteps issues with _the language itself_ in order to focus solely on low-level implementation details. IMO Adrian covered the remaining quite nicely. – Lightness Races in Orbit Aug 23 '19 at 10:43
There's also a bit of terminology ambiguity with "strings" vs "string literals" and "const" vs "immutable" but I think we can forgive that – Lightness Races in Orbit Aug 23 '19 at 10:45
"The only known place where this happens is the command line parameter in CreateProcessW." is *exactly* the problem I'm having which led me to this old post :). – Rick Henderson Jul 05 '23 at 17:58

Adrian Mole · Answer 2 · 2019-08-22T15:45:38.647

5

Be careful! As Lightness Races in Orbit points out, modifying the data you pass could be a problem down the road. Try this, instead:

MENUITEMINFO mii = { sizeof(mii) };
mii.fMask = MIIM_STRING | MIIM_FTYPE | MIIM_ID | MIIM_STATE;
mii.wID = idCmdFirst + IDM_DISPLAY;
mii.fType = MFT_STRING;
wchar_t text[] = L"&Sample Text";
mii.dwTypeData = text;
mii.fState = MFS_ENABLED;

This way, you should be a wee bit safer - but not completely!! As RbMm points out, it is far better as a general rule to have the dwTypeData member pointing to a static character array. Somewhere (outside) the function …

static thread_local wchar_t menuText[MAXTEXTLEN];

Then, set up mii with …

wcscpy(menuText, L"&Sample Text");
mii.dwTypeData = menuText;
mii.cch = MAXTEXTLEN; // But not used in this case!

edited Aug 22 '19 at 15:45

answered Aug 22 '19 at 14:59

Adrian Mole

49,934
160
51
83

1

Thanks. I will keep this method in mind for user code. For this specific case I think Anders answer would suffice, I hope. Sorry, I can't up vote anything yet. – workvact Aug 22 '19 at 15:04
1

but nobody try modify *mii.dwTypeData* here. simply winapi was designed before */Zc:strictStrings* compiler option – RbMm Aug 22 '19 at 15:15
@RbMm Absolutely agree **in this case**! But it's good practice to avoid passing string literals in structures like `MENUITEMINFO` as this can jump up and bite you on the behind, sometimes! – Adrian Mole Aug 22 '19 at 15:19
@Adrian - i can partial agree, if not performance penalty. in your example we need need allocate additional storage (usual in stack, because *text* will be usual local variable) and copy data from const literal to this temporary storage. also, not in this api of course, but from general view - some api can be asynchronous and require valid data until call not finished. in this case not posible use local variable and exit from function before api call end. – RbMm Aug 22 '19 at 15:24
no, i not say that have **static** storage better. really if multiple threads in concurrent call your function with static storage - will be race and bug. – RbMm Aug 22 '19 at 15:38
the `InsertMenuItem` is **synchronous** call - because this is full ok have temporary local buffer. simply from teoretical view, if we call asynchronous api - may be need more persistent storage, but not in this concrete case – RbMm Aug 22 '19 at 15:41
so we have choise - or formal follow strict c++ rules and not pass const data to pointer declared not as const and not use *const_cast* but lost in efficient, or use *const_cast* if we sure that data will be not modified and used as const, despite not declared as const – RbMm Aug 22 '19 at 15:44
1

@RbMm - Cool! See my edit to add `thread_local` after `static`. But there is not really a "right answer" here, it's down to the user/coder to choose what works best in each case. Using the WinAPI with strict, modern c++ will always present this sort of dilemma. – Adrian Mole Aug 22 '19 at 15:48
2

agree. i from self side prefer `const_cast` because understand that this is safe with **concrete** case/api. use permanent storage really need only in rarely case .. `static thread_local ` yet more overhead, when `InsertMenuItem` is exactly synchronous. so your first varian full ok here – RbMm Aug 22 '19 at 15:56
more interesting case if want [`LVITEMW`](https://learn.microsoft.com/en-us/windows/win32/api/commctrl/ns-commctrl-lvitemw) structure - *pszText is a pointer to a null-terminated string containing the item text. When responding to an LVN_GETDISPINFO notification, be sure that this pointer remains valid until after the next notification has been received.* but is only in case if we want change pointer of *pszText* to self string instead copy it to buffer – RbMm Aug 22 '19 at 15:59
so if we have constant string literal and want assign it to *pszText* When responding to an LVN_GETDISPINFO notification , instead of copy it to *pszText* here more interesting choice. – RbMm Aug 22 '19 at 16:01

How to initialize LPWSTR from a const string?

2 Answers2