[quoted text, click to view] Dilip wrote: > I have cross-posted this question to 3 newsgroups since I am not sure > exactly where it belongs (and some NG's have very less activity). > Apologies in advance for this. I have set the followup to the > dotnet.framework.interop NG but feel free to chage it as appopriate. > > I have a question regarding converting a Managed System.String^ to > unmanaged const char*. > I inherited a code that does it like this: > > String^ s = "00000"; > LPCSTR str = static_cast<LPCTSTR>(const_cast<void*>(static_cast<const > void*>(Marshal::StringToHGlobalAnsi(s))));
Abominable. One cast should raise a flag. Three casts should raise a red alert. This is clearly the work of someone who didn't know what they were doing and just strung casts together until it appeared to work. That said, this *does* happen to work, although it's unnecessarily cast-happy. It's equivalent to char* str = static_cast<char*>(Marshal::StringToHGlobalAnsi(s).ToPointer()); [quoted text, click to view] > char someStr[20]; > strncpy_s(someStr, 19, str, 19);
This call is wrong. The second argument to strncpy_s is the *total* size of the destination, including room for the null terminator. This call tries to copy 19 characters plus null terminator into a buffer with a specified size of 19 bytes, which will not fit. This should be strncpy_s(someStr, 20, str, 19); or even better strncpy_s(someStr, _countof(someStr), str, _TRUNCATE); to copy all the characters that will fit. If truncation is not supposed to happen, then just leave it at strncpy_s(someStr, _countof(someStr), str, strlen(str)); which will raise an error if the string does not fit. [quoted text, click to view] > someStr[19] = 0; > Marshal::FreeHGlobal(static_cast<IntPtr>(const_cast<void*>(static_cast<const > void*>(str)))); >
Here's a much shorter and cleaner way of doing the same thing: { pin_ptr<const wchar_t> ps = PtrToStringChars(s); size_t charsConverted; wcstombs_s(&charsConverted, someStr, _countof(someStr), ps, _TRUNCATE); } This copies as much characters of "s" as will fit into the array "someStr", converting the characters from Unicode to ANSI along the way. It does not allocate additional memory, which it achieves by temporarily pinning the string. Of course, depending on what you actually do with the string, copying to an array may be unnecessary in the first place. --
[quoted text, click to view] Jeroen Mostert wrote: > Dilip wrote: >> I have cross-posted this question to 3 newsgroups since I am not sure >> exactly where it belongs (and some NG's have very less activity). >> Apologies in advance for this. I have set the followup to the >> dotnet.framework.interop NG but feel free to chage it as appopriate. >> >> I have a question regarding converting a Managed System.String^ to >> unmanaged const char*. >> I inherited a code that does it like this: >> >> String^ s = "00000"; >> LPCSTR str = static_cast<LPCTSTR>(const_cast<void*>(static_cast<const >> void*>(Marshal::StringToHGlobalAnsi(s)))); > > Abominable. One cast should raise a flag. Three casts should raise a red > alert. This is clearly the work of someone who didn't know what they > were doing and just strung casts together until it appeared to work. > > That said, this *does* happen to work, although it's unnecessarily > cast-happy. It's equivalent to > > char* str = > static_cast<char*>(Marshal::StringToHGlobalAnsi(s).ToPointer()); >
Yeah, well, except for the fact that it should be const char* str = static_cast<const char*>(Marshal::StringToHGlobalAnsi(s).ToPointer()); but other than that... --
[quoted text, click to view] Dilip wrote: > On Feb 27, 3:48 pm, Jeroen Mostert <jmost...@xs4all.nl> wrote: <snip> > Gotcha Jeroen! Thanks for your suggestions. I accept your points but > just so that I can conclusively isolate this code as suspicious -- in > my specific case, 'str' comes in as "00000". so when you do: > > strncpy_s(someStr, 19, str, 19); > > 1) str is shorter than someStr, only 5 bytes and null-terminated (i.e > I am assuming the previous call to Marshal::StringToHGlobalAnsi > returns a pointer to a null terminated string). Isn't strncpy_s > supposed to stop copying characters when it encounters the null- > terminator?[1]
Yes, it does. The line is still wrong, of course, but if "str" really happens to be smaller, it will work. If the rest of the code looks like this, however, I wouldn't be surprised if it's corrupted the stack somewhere along the way, and you just happen to see the problem around here. Good luck diagnosing. [quoted text, click to view] > 2) _TRUNCATE is necessary only when source is greater than > destination, right? In my case that almost never happens. >
But if it does happen, you have to choose to either get an error or truncate the remainder, depending on what's most appropriate. --
I have cross-posted this question to 3 newsgroups since I am not sure exactly where it belongs (and some NG's have very less activity). Apologies in advance for this. I have set the followup to the dotnet.framework.interop NG but feel free to chage it as appopriate. I have a question regarding converting a Managed System.String^ to unmanaged const char*. I inherited a code that does it like this: String^ s = "00000"; LPCSTR str = static_cast<LPCTSTR>(const_cast<void*>(static_cast<const void*>(Marshal::StringToHGlobalAnsi(s)))); char someStr[20]; strncpy_s(someStr, 19, str, 19); someStr[19] = 0; Marshal::FreeHGlobal(static_cast<IntPtr>(const_cast<void*>(static_cast<const void*>(str)))); Leaving aside the fact that there are better ways to accomplish what I want (like calling .ToPointer() for example) can someone tell me if that piece of code as it stands has a potential to create problems? I am getting some System.AccessViolationException around these parts -- although I can't be sure this is what is causing the problem I want to eliminate it as a potential issue. Here are my issues with that block of code: 1) It seems to do an unnecessary amount of back-and-forth castings for what should essentially be cast to a void* and then to a const char*. First it casts the code to a const void*, then it casts away the const and then does a cast of const char*. I feel uneasy with so many casts around. 2) Same with the FreeHGlobal -- too many casts 3) the code that does strncpy seems to be copying more than it should. I think Marshal::StringTOHGlobalAnsi returns a null- terminated string. Copying 19 bytes further than seems to be trampling on memory that doesn't belong to the code.
[quoted text, click to view] "Dilip" <rdilipk@lycos.com> wrote in message news:ee20da4d-8975-40bc-8cd5-3a546b1ccacd@64g2000hsw.googlegroups.com... > > I have cross-posted this question to 3 newsgroups since I am not sure > exactly where it belongs (and some NG's have very less activity). > Apologies in advance for this. I have set the followup to the > dotnet.framework.interop NG but feel free to chage it as appopriate. > > I have a question regarding converting a Managed System.String^ to > unmanaged const char*. > I inherited a code that does it like this: > > String^ s = "00000"; > LPCSTR str = static_cast<LPCTSTR>(const_cast<void*>(static_cast<const > void*>(Marshal::StringToHGlobalAnsi(s)))); > char someStr[20]; > strncpy_s(someStr, 19, str, 19); > someStr[19] = 0; > Marshal::FreeHGlobal(static_cast<IntPtr>(const_cast<void*>(static_cast<const > void*>(str)))); > > Leaving aside the fact that there are better ways to accomplish what I > want (like calling .ToPointer() for example) can someone tell me if > that piece of code as it stands has a potential to create problems? I > am getting some System.AccessViolationException around these parts -- > although I can't be sure this is what is causing the problem I want to > eliminate it as a potential issue. Here are my issues with that block > of code: > > 1) It seems to do an unnecessary amount of back-and-forth castings for > what should essentially be cast to a void* and then to a const char*. > First it casts the code to a const void*, then it casts away the const > and then does a cast of const char*. I feel uneasy with so many casts > around. > 2) Same with the FreeHGlobal -- too many casts
The casts are necessary if using the casting operators. Using the old C-style casts it could be something like this: String^ s = "00000"; const char *str = (const char *)(void *)Marshal::StringToHGlobalAnsi(s); char someStr[20]; strncpy_s(someStr, 19, str, 19); someStr[19] = 0; Marshal::FreeHGlobal(System::IntPtr((void*)str)); [quoted text, click to view] > 3) the code that does strncpy seems to be copying more than it > should. I think Marshal::StringTOHGlobalAnsi returns a null- > terminated string. Copying 19 bytes further than seems to be > trampling on memory that doesn't belong to the code.
From the docs: "These functions try to copy the first D characters of strSource to strDest, where D is the lesser of count and the length of strSource." If you pass _TRUNCATE as the last parameter to strncpy_s, then you don't have to worry about "someStr[19] = 0;" strncpy_s(someStr, 20, str, _TRUNCATE); Which line are you getting the System.AccessViolationException on?? Mark -- Mark Salsbery Microsoft MVP - Visual C++ [quoted text, click to view] > > Any insights?
[quoted text, click to view] On Feb 27, 3:48=A0pm, Jeroen Mostert <jmost...@xs4all.nl> wrote: > Dilip wrote: > > char someStr[20]; > > strncpy_s(someStr, 19, str, 19); > > This call is wrong. The second argument to strncpy_s is the *total* size o= f > the destination, including room for the null terminator. This call tries t= o > copy 19 characters plus null terminator into a buffer with a specified siz= e > of 19 bytes, which will not fit. This should be > > strncpy_s(someStr, 20, str, 19); > > or even better > > strncpy_s(someStr, _countof(someStr), str, _TRUNCATE); > > to copy all the characters that will fit. If truncation is not supposed to= > happen, then just leave it at > > strncpy_s(someStr, _countof(someStr), str, strlen(str)); > > which will raise an error if the string does not fit.
Gotcha Jeroen! Thanks for your suggestions. I accept your points but just so that I can conclusively isolate this code as suspicious -- in my specific case, 'str' comes in as "00000". so when you do: strncpy_s(someStr, 19, str, 19); 1) str is shorter than someStr, only 5 bytes and null-terminated (i.e I am assuming the previous call to Marshal::StringToHGlobalAnsi returns a pointer to a null terminated string). Isn't strncpy_s supposed to stop copying characters when it encounters the null- terminator?[1] 2) _TRUNCATE is necessary only when source is greater than destination, right? In my case that almost never happens. [1] "The strncpy_s() function copies not more than a specified number of successive characters (characters that follow a null character are not copied) from a source string to a destination character array." https://buildsecurityin.us-cert.gov/daisy/bsi/articles/knowledge/coding/317.= html
On Feb 27, 3:32=A0pm, "Mark Salsbery [MVP]" [quoted text, click to view] <MarkSalsbery[MVP]@newsgroup.nospam> wrote: > "Dilip" <rdil...@lycos.com> wrote in message > > news:ee20da4d-8975-40bc-8cd5-3a546b1ccacd@64g2000hsw.googlegroups.com... > > > > > > > > > I have cross-posted this question to 3 newsgroups since I am not sure > > exactly where it belongs (and some NG's have very less activity). > > Apologies in advance for this. =A0I have set the followup to the > > dotnet.framework.interop NG but feel free to chage it as appopriate. > > > I have a question regarding converting a Managed System.String^ to > > unmanaged const char*. > > I inherited a code that does it like this: > > > String^ s =3D "00000"; > > LPCSTR str =3D static_cast<LPCTSTR>(const_cast<void*>(static_cast<const > > void*>(Marshal::StringToHGlobalAnsi(s)))); > > char someStr[20]; > > strncpy_s(someStr, 19, str, 19); > > someStr[19] =3D 0; > > Marshal::FreeHGlobal(static_cast<IntPtr>(const_cast<void*>(static_cast<c= ons=ADt > > void*>(str)))); > > > Leaving aside the fact that there are better ways to accomplish what I > > want (like calling .ToPointer() for example) can someone tell me if > > that piece of code as it stands has a potential to create problems? =A0I= > > am getting some System.AccessViolationException around these parts -- > > although I can't be sure this is what is causing the problem I want to > > eliminate it as a potential issue. =A0Here are my issues with that block= > > of code: > > > 1) It seems to do an unnecessary amount of back-and-forth castings for > > what should essentially be cast to a void* and then to a const char*. > > First it casts the code to a const void*, then it casts away the const > > and then does a cast of const char*. =A0I feel uneasy with so many casts= > > around. > > 2) Same with the FreeHGlobal -- too many casts > > The casts are necessary if using the casting operators. =A0Using the old > C-style casts it could be something like =A0 this: > > String^ s =3D "00000"; > const char *str =3D (const char *)(void *)Marshal::StringToHGlobalAnsi(s);= > char someStr[20]; > strncpy_s(someStr, 19, str, 19); > someStr[19] =3D 0; > Marshal::FreeHGlobal(System::IntPtr((void*)str)); > > > 3) the code that does strncpy seems to be copying more than it > > should. =A0I think Marshal::StringTOHGlobalAnsi returns a null- > > terminated string. =A0Copying 19 bytes further than seems to be > > trampling on memory that doesn't belong to the code. > > From the docs: > "These functions try to copy the first D characters of strSource to strDes= t, > where D is the lesser of count and the length of strSource." > > If you pass _TRUNCATE as the last parameter to strncpy_s, then you don't > have to worry about "someStr[19] =3D 0;" > > strncpy_s(someStr, 20, str, _TRUNCATE); > > Which line are you getting the System.AccessViolationException =A0on??
Thanks Mark. The exact line isn't captured by the debugger. the control lands at a place that is few lines below the suspicious code I wrote about. I have responded to Jeroen's post. Do let me know what
[quoted text, click to view] > Leaving aside the fact that there are better ways to accomplish what I > want (like calling .ToPointer() for example)
If you know that is better, why not use it? ======================== String^ managedString = "Hello unmanaged world (from the managed world)."; char* stringPointer = (char*) Marshal::StringToHGlobalAnsi(managedString ).ToPointer(); Marshal::FreeHGlobal(IntPtr(stringPointer)); Official example at http://msdn2.microsoft.com/en- us/library/system.runtime.interopservices.marshal.stringtohglobalansi.aspx ================== [quoted text, click to view] > LPCSTR str = static_cast<LPCTSTR>(const_cast<void*>(static_cast<const
Potential problem here: why static_cast to LPCTSTR if what you want is LPCSTR? The 'T' in LPCTSTR means that when compiled as Unicode the type is really wchar_t const *, while LPCSTR is char const * So you basically do char const * str = static_cast<wchar_t const *>(...) -- Mihai Nita [Microsoft MVP, Windows - SDK] http://www.mihai-nita.net ------------------------------------------
[quoted text, click to view] > strncpy_s(someStr, 19, str, 19); > someStr[19] = 0;
Extra: there is no need to "manually" put the ending zero if you use strncpy_s -- Mihai Nita [Microsoft MVP, Windows - SDK] http://www.mihai-nita.net ------------------------------------------
[quoted text, click to view] "Mihai N." <nmihai_year_2000@yahoo.com> wrote in message news:Xns9A51BD2DAA573MihaiN@207.46.248.16... > >> Leaving aside the fact that there are better ways to accomplish what I >> want (like calling .ToPointer() for example) > If you know that is better, why not use it? > > ======================== > String^ managedString = "Hello unmanaged world (from the managed world)."; > char* stringPointer = (char*) Marshal::StringToHGlobalAnsi(managedString > ).ToPointer(); > Marshal::FreeHGlobal(IntPtr(stringPointer));
Don't cast and cast back. Instead: IntPtr hglobalPtr = Marshal::StringToHGlobalAnsi(managedString); char* stringPointer = static_cast<char*>(hglobalPtr.ToPointer()); .... Marshal::FreeHGlobal(hglobalPtr); [quoted text, click to view] > > Official example at http://msdn2.microsoft.com/en- > us/library/system.runtime.interopservices.marshal.stringtohglobalansi.aspx > ================== > > >> LPCSTR str = static_cast<LPCTSTR>(const_cast<void*>(static_cast<const > Potential problem here: why static_cast to LPCTSTR if what you want is > LPCSTR? The 'T' in LPCTSTR means that when compiled as Unicode the type > is really wchar_t const *, while LPCSTR is char const * > So you basically do char const * str = static_cast<wchar_t const *>(...) > > > -- > Mihai Nita [Microsoft MVP, Windows - SDK] > http://www.mihai-nita.net > ------------------------------------------ > Replace _year_ with _ to get the real email
The only problem that I see with the casts is that they're unreadable sequences of garbage, but no obvious technical problems. Your insight about the number of bytes is correct. The program tramples on 14 unknown bytes. We can only wonder why the program sometimes works. [quoted text, click to view] "Dilip" <rdilipk@lycos.com> wrote in message news:ee20da4d-8975-40bc-8cd5-3a546b1ccacd@64g2000hsw.googlegroups.com... > > I have cross-posted this question to 3 newsgroups since I am not sure > exactly where it belongs (and some NG's have very less activity). > Apologies in advance for this. I have set the followup to the > dotnet.framework.interop NG but feel free to chage it as appopriate. > > I have a question regarding converting a Managed System.String^ to > unmanaged const char*. > I inherited a code that does it like this: > > String^ s = "00000"; > LPCSTR str = static_cast<LPCTSTR>(const_cast<void*>(static_cast<const > void*>(Marshal::StringToHGlobalAnsi(s)))); > char someStr[20]; > strncpy_s(someStr, 19, str, 19); > someStr[19] = 0; > Marshal::FreeHGlobal(static_cast<IntPtr>(const_cast<void*>(static_cast<const > void*>(str)))); > > Leaving aside the fact that there are better ways to accomplish what I > want (like calling .ToPointer() for example) can someone tell me if > that piece of code as it stands has a potential to create problems? I > am getting some System.AccessViolationException around these parts -- > although I can't be sure this is what is causing the problem I want to > eliminate it as a potential issue. Here are my issues with that block > of code: > > 1) It seems to do an unnecessary amount of back-and-forth castings for > what should essentially be cast to a void* and then to a const char*. > First it casts the code to a const void*, then it casts away the const > and then does a cast of const char*. I feel uneasy with so many casts > around. > 2) Same with the FreeHGlobal -- too many casts > 3) the code that does strncpy seems to be copying more than it > should. I think Marshal::StringTOHGlobalAnsi returns a null- > terminated string. Copying 19 bytes further than seems to be > trampling on memory that doesn't belong to the code. > > Any insights?
Don't see what you're looking for? Try a search.
|