std::string <--> System::String*

  • Thread starter Thread starter Marcus Kwok
  • Start date Start date
M

Marcus Kwok

Hello, I am working on cleaning up some code that I inherited and was
wondering if there is anything wrong with my function. I am fairly
proficient in standard C++ but I am pretty new to the .NET managed C++.
It seems to work fine, but everyone knows that programs with errors can
still appear to "work fine" :)

I am working with VS .NET 2003; I am unable to upgrade to 2005 at this
time, so I cannot use the newer syntax or features.

This function takes a std::string and converts it to a System::String*.


Here is the original:

System::String*
MarshalStdToNetString(std::string& ss)
{
if (ss.empty())
return new System::String(S"");

System::IntPtr ptr(static_cast<System::IntPtr>(static_cast<void*>(const_cast<char*>(ss.c_str()))));

System::String* ret(System::Runtime::InteropServices::Marshal::PtrToStringAnsi(ptr));

return ret;
}


Here is my updated version:
// function name is just shorthand for what I will eventually call it

System::String*
std2gc(const std::string& s)
{
return new System::String(s.c_str());
}


Have I oversimplified? Or is all that casting business really needed?


Also, going in the other direction, is this the proper way to convert a
managed System::String* into a std::string? I haven't started trying to
change this one yet:

void
MarshalNetToStdString(System::String* s, std::string& os)
{
using System::IntPtr;
using System::Runtime::InteropServices::Marshal;

const char* chars = (const char*)(Marshal::StringToHGlobalAnsi(s)).ToPointer();
os = chars;
Marshal::FreeHGlobal(IntPtr((void*)chars));
}
 
Hi Marcus!
Here is my updated version:
// function name is just shorthand for what I will eventually call it

System::String*
std2gc(const std::string& s)
{
return new System::String(s.c_str());
}


Have I oversimplified?
Click to expand...

No. That look´s really good!

Also, going in the other direction, is this the proper way to convert a
managed System::String* into a std::string? I haven't started trying to
change this one yet:

void
MarshalNetToStdString(System::String* s, std::string& os)
{
using System::IntPtr;
using System::Runtime::InteropServices::Marshal;

const char* chars = (const char*)(Marshal::StringToHGlobalAnsi(s)).ToPointer();
os = chars;
Marshal::FreeHGlobal(IntPtr((void*)chars));
}
Click to expand...

It looks ok...

--
Greetings
Jochen

My blog about Win32 and .NET
http://blog.kalmbachnet.de/
 
Jochen Kalmbach said:
Hi Marcus!


No. That look?s really good!
Click to expand...

OK, thanks!
It looks ok...
Click to expand...

Thanks, after looking around some more I found almost identical code in
MSDN, so I think this is the "right" way to do it, though maybe I'll
change it to return the std::string instead of passing an out-parameter
to the function.
 
Marcus Kwok said:
This function takes a std::string and converts it to a
System::String*.
Click to expand...

you should also think about std::wstring conversions; potentially these
could be faster, as CLI String is unicode one which means there would't
be conversion in String constructor and marshaller, and you would't lose
data (anything beyond ASCII)


B.
 
Bronek Kozicki said:
you should also think about std::wstring conversions; potentially these
could be faster, as CLI String is unicode one which means there would't
be conversion in String constructor and marshaller, and you would't lose
data (anything beyond ASCII)
Click to expand...

Thanks for the idea. However, this code is in a GUI that just passes
the strings to a backend calculation engine, so conversion speed is not
an issue. Also, all of our data is plain ASCII so I don't need to
support Unicode.
 
Marcus Kwok said:
Also, going in the other direction, is this the proper way to convert a
managed System::String* into a std::string? I haven't started trying to
change this one yet:

void
MarshalNetToStdString(System::String* s, std::string& os)
{
using System::IntPtr;
using System::Runtime::InteropServices::Marshal;

const char* chars = (const char*)(Marshal::StringToHGlobalAnsi(s)).ToPointer();
os = chars;
Marshal::FreeHGlobal(IntPtr((void*)chars));
}
Click to expand...

Here is my new implementation. Personally I feel this is a little
cleaner since it eliminates a few casts and the interface is more
consistent with the other (snipped) function, but it is essentially the
same:

std::string
gc2std(System::String* s)
{
using System::IntPtr;
using System::Runtime::InteropServices::Marshal;

IntPtr ip = Marshal::StringToHGlobalAnsi(s);
std::string ss = static_cast<const char*>(ip.ToPointer());
Marshal::FreeHGlobal(ip);
return ss;
}
 
Marcus Kwok said:
std::string
gc2std(System::String* s)
{
using System::IntPtr;
using System::Runtime::InteropServices::Marshal;

IntPtr ip = Marshal::StringToHGlobalAnsi(s);
std::string ss = static_cast<const char*>(ip.ToPointer());
Marshal::FreeHGlobal(ip);
return ss;
}
Click to expand...

nice, however I see slight exception safete problem here : std::string
constructor might throw std::bad_alloc (eg. due to high memory
fragmentation and/or very long string being constructed) and in such
situation Marshal::FreeHGlobal will not be called.


B.
 
Bronek Kozicki said:
nice, however I see slight exception safete problem here : std::string
constructor might throw std::bad_alloc (eg. due to high memory
fragmentation and/or very long string being constructed) and in such
situation Marshal::FreeHGlobal will not be called.
Click to expand...

Ahh, yes, good point. This is exactly the kind of feedback I was
looking for :) I need to work on improving my exception-safety
abilities.

So, is this exception-safe? Although I vaguely recall reading somewhere
that duplicating the clean-up code like this is considered bad style. I
guess this is why the original version used the output parameter
(reference) to save the string, but I wonder if the assignment operator
in the original could cause the same issue.


std::string
gc2std(System::String* s)
{
using System::IntPtr;
using System::Runtime::InteropServices::Marshal;

IntPtr ip = Marshal::StringToHGlobalAnsi(s);

std::string ss;
try {
ss = static_cast<const char*>(ip.ToPointer());
}
catch (std::bad_alloc&) {
Marshal::FreeHGlobal(ip);
throw;
}

Marshal::FreeHGlobal(ip);

return ss;
}
 
Hi Marcus!
try {
ss = static_cast<const char*>(ip.ToPointer());
}
catch (std::bad_alloc&) {
Marshal::FreeHGlobal(ip);
throw;
}

Marshal::FreeHGlobal(ip);
Click to expand...

I suggest to use "try-finally" and remove the "using System::IntPtr;":

String *s = S"abc";
using System::Runtime::InteropServices::Marshal;
IntPtr ip = Marshal::StringToHGlobalAnsi(s);
std::string ss;
try
{
ss = static_cast<const char*>(ip.ToPointer());
}
__finally
{
Marshal::FreeHGlobal(ip);
}
return ss;



--
Greetings
Jochen

My blog about Win32 and .NET
http://blog.kalmbachnet.de/
 
Better would be:

void Conv(System::String *s, std::string &ss)
{
using System::Runtime::InteropServices::Marshal;
IntPtr ip = Marshal::StringToHGlobalAnsi(s);
ss.clear();
try
{
ss = static_cast<const char*>(ip.ToPointer());
}
__finally
{
Marshal::FreeHGlobal(ip);
}
}

So the memory will only be allocated once...

--
Greetings
Jochen

My blog about Win32 and .NET
http://blog.kalmbachnet.de/
 
Better would be:
Click to expand...

Ok, last try:

void Conv(System::String *s, std::string &ss)
{
ss.clear();
if (s == 0) return;
using System::Runtime::InteropServices::Marshal;
IntPtr ip = Marshal::StringToHGlobalAnsi(s);
try
{
ss = static_cast<const char*>(ip.ToPointer());
}
__finally
{
Marshal::FreeHGlobal(ip);
}
}



--
Greetings
Jochen

My blog about Win32 and .NET
http://blog.kalmbachnet.de/
 
Knowing nothing of managed / CLR would the following not suffice
Ok, last try:

void Conv(System::String *s, std::string &ss)
{
ss.clear();
if (s == 0) return;
using System::Runtime::InteropServices::Marshal;
Click to expand...

struct SafeStringAccessor
{
IntPtr mIp;

SafePtr( System::String* s ):mIp(Marshal::StringToHGlobalAnsi(s)){}
~SafePtr(){ mIp(Marshal::FreeHGlobal(mIp); }

const char* c_str()const
{
return static_cast<const char*>(mIp.ToPointer());
}
}

ss = SafeStringAccessor(s).c_str();
}
Click to expand...

Of course with the idea of parameterizing this and/or using
boost::shared_ptr with an appropriate deleter.

Thanks, Jeff Flinn
 
Jeff F said:
Knowing nothing of managed / CLR would the following not suffice


struct SafeStringAccessor
{
IntPtr mIp;

SafePtr( System::String* s ):mIp(Marshal::StringToHGlobalAnsi(s)){}
~SafePtr(){ mIp(Marshal::FreeHGlobal(mIp); }

const char* c_str()const
{
return static_cast<const char*>(mIp.ToPointer());
}
}

ss = SafeStringAccessor(s).c_str();

Of course with the idea of parameterizing this and/or using
boost::shared_ptr with an appropriate deleter.
Click to expand...

Yes, I also do prefer the RAII idiom over explicit try/catch blocks, and
I have been doing mainly ISO C++ and not very much .NET stuff so I
didn't know about the __finally keyword.


I had to modify some things to get it to compile under /clr:

(I do not like to use using directives, and my compiler is telling me
that I cannot add a using declaration for Marshal, which is why I
fully-qualify the name, and also apparently you can't use the 'const'
qualifier on managed types)


__gc class SafeStringAccessor {
public:
SafeStringAccessor(System::String* s) : ip_(System::Runtime::InteropServices::Marshal::StringToHGlobalAnsi(s)) { }

~SafeStringAccessor() { System::Runtime::InteropServices::Marshal::FreeHGlobal(ip_); }

const char* c_str()
{
return static_cast<const char*>(ip_.ToPointer());
}
private:
System::IntPtr ip_;
};


std::string ss = (new SafeStringAccessor(s))->c_str();


But now this is starting to look a little bit messier than the
stand-alone functions.
 
Hi Marcus!
__gc class SafeStringAccessor {
Click to expand...

You shouldn´t declare it as managed type... this might lead to leaks!
Because you have a undeterministic call of the finalizer (destructor) in
the managed world!!


The following should be correct:

struct Conv
{
char *szAnsi;
Conv(System::String* s)
:
szAnsi(static_cast<char*>(System::Runtime::InteropServices::Marshal::StringToHGlobalAnsi(s).ToPointer()))
{}
~Conv()
{

System::Runtime::InteropServices::Marshal::FreeHGlobal(IntPtr(szAnsi));
}
const char* c_str() const
{
return szAnsi;
}
};


int _tmain()
{
String *s = S"abc";
std::string ss = Conv(s).c_str();
return 0;
}



public:
SafeStringAccessor(System::String* s) : ip_(System::Runtime::InteropServices::Marshal::StringToHGlobalAnsi(s)) { }

~SafeStringAccessor() { System::Runtime::InteropServices::Marshal::FreeHGlobal(ip_); }

const char* c_str()
{
return static_cast<const char*>(ip_.ToPointer());
}
private:
System::IntPtr ip_;
};


std::string ss = (new SafeStringAccessor(s))->c_str();


But now this is starting to look a little bit messier than the
stand-alone functions.
Click to expand...


--
Greetings
Jochen

My blog about Win32 and .NET
http://blog.kalmbachnet.de/
 
Jochen said:
Hi Marcus!

You shouldn´t declare it as managed type... this might lead to leaks!
Because you have a undeterministic call of the finalizer (destructor)
in the managed world!!


The following should be correct:

struct Conv
{
char *szAnsi;
Conv(System::String* s)
:
szAnsi(static_cast<char*>(System::Runtime::InteropServices::Marshal::StringToHGlobalAnsi(s).ToPointer()))
{}
~Conv()
{

System::Runtime::InteropServices::Marshal::FreeHGlobal(IntPtr(szAnsi));
}
const char* c_str() const
{
return szAnsi;
}
};
Click to expand...

and to further simplify:


std::string StdStringFrom( System::String* s)
{
return Conv(s).c_str();
}
int _tmain()
{
String *s = S"abc";
Click to expand...

std::string ss = StdStringFrom(s);
return 0;
}
Click to expand...

Jeff Flinn
 
Jeff F said:
and to further simplify:


std::string StdStringFrom( System::String* s)
{
return Conv(s).c_str();
}


std::string ss = StdStringFrom(s);
Click to expand...

Thank you everybody. I think this version is the cleanest (except I
made szAnsi private in the Conv struct/class).
 
Marcus Kwok said:
Thank you everybody. I think this version is the cleanest (except I
made szAnsi private in the Conv struct/class).
Click to expand...

So, to summarize, here is the final implementation I have decided on,
barring any other issues with this code. I especially like the symmetry
between the two Marshal* functions.


#ifndef MARSHAL_STRINGS_H
#define MARSHAL_STRINGS_H

#include <string>

#using <mscorlib.dll>

class Conv {
public:
Conv(System::String* s)
: cp(static_cast<char*>(System::Runtime::InteropServices::Marshal::StringToHGlobalAnsi(s).ToPointer()))
{ }

~Conv()
{
System::Runtime::InteropServices::Marshal::FreeHGlobal(static_cast<System::IntPtr>(cp));
}

const char* c_str() const
{
return cp;
}

private:
char* cp;
};



// declared inline so I can keep the implementation in a header file
// without getting linker errors
inline
std::string
MarshalNetToStdString(System::String* s)
{
return std::string(Conv(s).c_str());
}



// declared inline so I can keep the implementation in a header file
// without getting linker errors
inline
System::String*
MarshalStdToNetString(const std::string& s)
{
return new System::String(s.c_str());
}

#endif
 
Hi Marcus!

Why not use:

struct Conv
{
char *szAnsi;
Conv(System::String* s)
:
szAnsi(static_cast<char*>(System::Runtime::InteropServices::Marshal::StringToHGlobalAnsi(s).ToPointer()))
{}
~Conv()
{
System::Runtime::InteropServices::Marshal::FreeHGlobal(IntPtr(szAnsi));
}
operator LPCSTR() const
{
return szAnsi;
}
};


Then you can simply write:


std::string ss = Conv(s);

--
Greetings
Jochen

My blog about Win32 and .NET
http://blog.kalmbachnet.de/
 
Jochen Kalmbach said:
Hi Marcus!

Why not use:

struct Conv
{
char *szAnsi;
Conv(System::String* s)
:
szAnsi(static_cast<char*>(System::Runtime::InteropServices::Marshal::StringToHGlobalAnsi(s).ToPointer()))
{}
~Conv()
{
System::Runtime::InteropServices::Marshal::FreeHGlobal(IntPtr(szAnsi));
}
operator LPCSTR() const
{
return szAnsi;
}
};


Then you can simply write:

std::string ss = Conv(s);
Click to expand...

Nice, now this eliminates the need for a second wrapper function :)


class MarshalNetToStdString {
public:
MarshalNetToStdString(System::String* s)
: cp(static_cast<char*>(System::Runtime::InteropServices::Marshal::StringToHGlobalAnsi(s).ToPointer()))
{ }

~MarshalNetToStdString()
{
System::Runtime::InteropServices::Marshal::FreeHGlobal(static_cast<System::IntPtr>(cp));
}

operator const char*() const
{
return cp;
}

private:
char* cp;

MarshalNetToStdString(const MarshalNetToStdString&);
MarshalNetToStdString& operator=(const MarshalNetToStdString&);
};


However, thinking about it, I was reminded of the "rule of 3" (in
essence, it says that if you provide any of {destructor, copy
constructor, assignment operator} then you probably need to provide all
three of them). I felt that it wouldn't really make much sense to be
copying these objects around, so I made the copy constructor and
assignment operator private to prevent the compiler from generating
default ones that will only perform shallow copies. The default
constructor does not get auto-generated since I have a user-defined
constructor, so I don't have to worry about that one.


Please see if the following train of thought sounds logical:
I only intend to use this class in the context of

System::String* s = new System::String("...");
std::string ss = MarshalNetToStdString(s);

In other words, create a temporary object (which gets destroyed at the
end of the statement) to do the conversion. In order to avoid the
overhead of creating/destroying an object every time, I thought about
making static member functions to do the conversion, but then I realized
that this would make the memory management much more complex, so I
decided against it. Plus, this class seems pretty lightweight so I
don't think the overhead is too great compared to the safety it
provides.
 
Jochen said:
Hi Marcus!

Why not use:

struct Conv
{
char *szAnsi;
Conv(System::String* s)
:
szAnsi(static_cast<char*>(System::Runtime::InteropServices::Marshal::StringToHGlobalAnsi(s).ToPointer()))
{}
~Conv()
{
System::Runtime::InteropServices::Marshal::FreeHGlobal(IntPtr(szAnsi));
}
operator LPCSTR() const
{
return szAnsi;
}
};


Then you can simply write:


std::string ss = Conv(s);
Click to expand...

This approach was discarded(by me) primarily to avoid the nasty surpises
that go along with implicit conversions, which are the same reasons for
std::string::c_str().

Jeff Flinn
 
Back
Top