 |
 | Home | Contact Us | FAQ | Search & Site Map | Link to Us |
 | Sign In | Join | Other 45 Sites in Network |
.NET Forum / .NET Framework / Interop / March 2008Marshal::StringToHGlobalAnsi and System.AccessViolationException
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? > 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). [quoted text clipped - 28 lines] > 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)); > 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  SignatureMark Salsbery Microsoft MVP - Visual C++ > Any insights? On Feb 27, 3:32 pm, "Mark Salsbery [MVP]" <MarkSalsbery[MVP]@newsgroup.nospam> 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). [quoted text clipped - 54 lines] > > Which line are you getting the System.AccessViolationException on?? 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 you think. > 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). [quoted text clipped - 8 lines] > 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()); > 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. > 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. SignatureJ. >> 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). [quoted text clipped - 18 lines] > 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... SignatureJ. > > char someStr[20]; > > strncpy_s(someStr, 19, str, 19); [quoted text clipped - 16 lines] > > 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 <snip> > Gotcha Jeroen! Thanks for your suggestions. I accept your points but > just so that I can conclusively isolate this code as suspicious -- in [quoted text clipped - 7 lines] > 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. > 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. SignatureJ. > 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 ================== > 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 *>(...) SignatureMihai Nita [Microsoft MVP, Windows - SDK] http://www.mihai-nita.net ------------------------------------------ Replace _year_ with _ to get the real email >> Leaving aside the fact that there are better ways to accomplish what I >> want (like calling .ToPointer() for example) [quoted text clipped - 5 lines] > ).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); > Official example at http://msdn2.microsoft.com/en- > us/library/system.runtime.interopservices.marshal.stringtohglobalansi.aspx [quoted text clipped - 5 lines] > is really wchar_t const *, while LPCSTR is char const * > So you basically do char const * str = static_cast<wchar_t const *>(...) > 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 SignatureMihai 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. > 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). [quoted text clipped - 34 lines] > > Any insights?
Free MagazinesGet these publications absolutely FREE for up to 12 months. There are no hidden fees and no obligation. Simply choose a title, complete the application form and submit it. Read more ...
|
Reply to this Message
|
 Sign In
 Join
 My Latest Posts My Monitored Threads My Blog My Photo Gallery My Profile My Homepage Start New Thread Enable EMail Alerts Rate this Thread
|
©2008 Advenet LLC Privacy Policy - Terms of Use
This website includes both content owned or controlled by Advenet as well as content owned or controlled by third parties.