2

How to copy memory bytes to a string variable assigned to a class object with memory leak? The memory holds bytes to be copied (not ends with '\0'), but will be freed later.

The example code is below, and Valgrind all reports memory leak for three cases.

Update: I forget to mention the string variable will be used by another class object.

class Bar
{
   string name;

   ~Bar()
   {
     // should we manually free name's buffer in Bar destructor?    
     // name's buffer is possibly on heap, see Valgrind message
   }
}

void foo(unsigned char *data, uint32 len, Bar *bar)
{
   string str;
   
   // case1, direct conversion
   str = string((char *)data);  
   
   // case2, allocate a new buffer and then copy
   unsigned char *p;            
   p = new unsigned char [len];
   memcpy(p, data, len);
   str = string(char *)p);
   delete[] p;

   // case3, use assign
   str.assign(data, len);   

   // memory leak
   bar->name = str;   
   
}

Valgrind message:

==65558== 96 bytes in 2 blocks are definitely lost in loss record 55 of 128
==65558==    at 0x10012B532: malloc (in /usr/local/valgrind/vgpreload_memcheck-amd64-darwin.so)
==65558==    by 0x1006C3DE0: operator new(unsigned long) (in /usr/lib/libc++abi.dylib)
==65558==    by 0x10063531B: std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::__grow_by_and_replace(unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, char const*) (in /usr/lib/libc++.1.dylib)
==65558==    by 0x100634E75: std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::assign(char const*, unsigned long) (in /usr/lib/libc++.1.dylib)

Thank you.

Blade
  • 55
  • 5
  • my guess is that you allocated the argument data on the heap before calling this function and have not freed it. This code is fine (as long as data is 0 terminated) – pm100 Jan 28 '22 at 05:42
  • 1
    Is the data guaranteed to be '\0' terminated? Also don't use "C" style casts like (char *), but if you must use static_cast(data); And any solution with new/delete should be considered last (new/delete are no longer the recommended solution in C++) – Pepijn Kramer Jan 28 '22 at 05:43
  • @pm100 thanks. data is a large memory chunk and it does not end with '\0'. – Blade Jan 28 '22 at 05:48
  • 2
    If it's not null terminated, you must use the provided len argument. std::string has a constructor for that. – Kenny Ostrom Jan 28 '22 at 05:52
  • 1
    well not ending with 0 makes case1 bad then, that will copy until it hits 0. – pm100 Jan 28 '22 at 05:52
  • valgrind will point at the line where the leaked allocation was made, what is it pointing at? Also note that the C++ runtime string library doesnt release string memory back to the heap. So maybe not a real leak. You can tell GCC to have an honest string heap https://stackoverflow.com/questions/1901322/valgrind-reports-memory-leak-when-assigning-a-value-to-a-string – pm100 Jan 28 '22 at 05:53
  • 1
    I assume you meant to ask how to copy memory bytes **without** a memory leak, i.e. that the first sentence in the question has an error and isn't asking how to create a memory leak? – Jeremy Friesner Jan 28 '22 at 06:04
  • The most important attribute you've is the `length` information of the given buffer. You should definitely use it under any condition. Don't rely on the `null` termination method. – Caglayan DOKME Jan 28 '22 at 06:10
  • Please show a [mre] – Alan Birtles Jan 28 '22 at 06:13

1 Answers1

0

The easiest way to construct a std::string from unterminated data is to use the constructor that takes both a pointer and a length:

string str(reinterpret_cast<const char *>(data), len);

Once you've done that, you can pass around str however you wish without having to worry about leaking memory (str will have made its own internal copy of the data and will take care not to leak it).

Jeremy Friesner
  • 70,199
  • 15
  • 131
  • 234