3 Quick Memory-Related Questions

[freehat]

I'm a little confused about cleaning up after myself. Let's assume I have the following situation (regardless of whether it makes sense or not):

char* convert_str( char* type_a, unsigned int type_a_len )
{
   char* converted_result = ( char* )malloc( sizeof( char ) * type_a_len );
   //perform conversion
   return converted_result;
}

void test( map< string, int >* p_map, string* p_unconverted_str )
{
   string str( convert_str( p_unconverted_str->c_str(), p_unconverted_str->length() );
   ( *p_map )[ str ] = str.length();
}

I have 3 questions:


1) Is the malloc'd string automatically freed when string str goes out of scope or do I still have to call:

free( str.c_str() );?

2) Would it be better to use

auto_ptr< char > ap_str( convert_str( ... ) );?

Or does auto_ptr rely on delete and it's not good to mix delete and free?

My last question is, if we substitute this instead:

void test( vector< string >* p_vector, string* p_unconverted_str )
{
   string str( convert_str( p_unconverted_str->c_str(), p_unconverted_str->length() );
   p_map->push_back( str );
}

3) Does vector use the copy constructor of string to create its own copy or if I use free( str.c_str() ); will the vector now point to a bad pointer?

[Disch]

Is the malloc'd string automatically freed when string str goes out of scope

No.

do I still have to call:

free( str.c_str() );?

This is also incorrect. You did not malloc str.c_str -- so you shouldn't free it. You malloc'd converted_result, so that's the buffer you'd need to free.

Since you are passing converted_result directly to str's constructor, you have no opportunity to free it and are therefore leaking memory.

To prevent the leak you'd need to do something like this:

char* tempbuf = convert_str( p_unconverted_str->c_str(), p_unconverted_str->length() );
string str( tempbuf );
free( tempbuf );  // free it now that we're done with it

( *p_map )[ str ] = str.length();

Note that 'str' doesnt' use tempbuf internally. It copies it into its own internal buffer. So we can free tempbuf immediately after we give it to str.

str will free its own copy, so we don't need to worry about cleaning up str. But it will not clean up tempbuf.

Would it be better to use

auto_ptr< char >

No. auto_ptr only works when you use singular new (ie: not new[], not malloc), so you can't use it in this case.

Really, it's better to just use strings and not bother with char pointers at all.

Does vector use the copy constructor of string to create its own copy

Yes.

You don't have to worry about what vector and string do. They clean up after themselves. You should only worry about cleaning up things you allocate yourself.

if I use free( str.c_str() );

Again -- you must NEVER do this.



As I previously mentioned, it's best if you just avoid char arrays completely. They just make things complicated. The best way to do this would be like this:

string convert_str( const string& type_a )
{
   string converted_result;

   // perform conversion

   return converted_result;
}

void test( map< string, int >* p_map, string* p_unconverted_str )
{
   string str( convert_str( *p_unconverted_str ) );
   ( *p_map )[ str ] = str.length();
}

Let string take care of the memory management. That way you dont' have to worry about anything.



Also, malloc has no place in C++.

[freehat]

Thanks! That clears up so much. I think I'm just making it harder than it is.


I just have one follow-up question:


It might be a misconception from my time with C, but it looks like you're passing the value of string in and then passing string out by value. Does this mean that 2 copies are necessary? 1) on entering function, 2) on exiting function?

If so, would this be okay?

string convert_str( const string* type_a )
{
   string converted_result;

   // perform conversion

   return converted_result;
}

void test( map< string, int >* p_map, string* p_unconverted_str  )
{
   string converted_str( convert_str( p_unconverted_str ) );
   //no memory management neded
}

[Disch]

It might be a misconception from my time with C,

That explains why you seem to want to use pointers everywhere.

It really isn't necessary in C++. In fact, try to avoid it. It just makes it harder.

but it looks like you're passing the value of string in and then passing string out by value. Does this mean that 2 copies are necessary?

I'm passing the string by const reference. So there is no copy going in.

The string is being passed out by value, so there might be a copy or there might not be. Compilers can optimize away the copy (or, if using the new C++ standard, can replace it with a move). In short: don't worry about it.

If so, would this be okay?

Yes that would also work, but demands you use pointers.

You wouldn't be able to do something like this, in that case:

string foo = convert_str( "foo" );  // would work with my function, but not with yours
  // since "foo" can be constructed as a std::string, but cannot be a pointer to a std::string.