CODEWITHSUNDEEP
c++stringvectorvalgrind
user4081979
user4081979

Reputation: 3

Storing deep copies of std::string data into std::vector

I must be going crazy but I'm following the examples from Convert std::string to const char* or char* and I'm getting a lot of Invalid read of size 1 in valgrind. I'm basically trying split whitespace delimited strings and store them into a std::vector<const char*>. I know that because pointers returned from c_str() can be invalidated at any moment, that you have to perform a copy in order to use the data safely. But it seems that I must be missing something since valgrind is complaining. At the absolute minimum, I'm copying the top answer and doing this:

#include <iostream>
#include <sstream>
#include <vector>
using namespace std;

int main()
{
    std::vector<const char*> args;

    std::string str = "hey";

    char * writable = new char[str.size() + 1];
    std::copy(str.begin(), str.end(), writable);
    writable[str.size()] = '\0'; // don't forget the terminating 0
    args.push_back(writable);
    delete[] writable;

    for (size_t i = 0; i < args.size(); ++i)
        cout << args[i] << "\n";

    return 0;
}

Which results in the following output from valgrind:

==5297== Invalid read of size 1
==5297==    at 0x4C2BFC2: __GI_strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5297==    by 0x400CFF: main (in /tmp/1411705143.17426/a.out)
==5297==  Address 0x5c2a0a0 is 0 bytes inside a block of size 4 free'd
==5297==    at 0x4C2A09C: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5297==    by 0x400CD9: main (in /tmp/1411705143.17426/a.out)
==5297== 
==5297== Invalid read of size 1
==5297==    at 0x4C2BFD4: __GI_strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5297==    by 0x400CFF: main (in /tmp/1411705143.17426/a.out)
==5297==  Address 0x5c2a0a1 is 1 bytes inside a block of size 4 free'd
==5297==    at 0x4C2A09C: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5297==    by 0x400CD9: main (in /tmp/1411705143.17426/a.out)
==5297== 
==5297== Invalid read of size 1
==5297==    at 0x58E6DB8: _IO_default_xsputn (genops.c:480)
==5297==    by 0x58E4D19: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1393)
==5297==    by 0x58DA98C: fwrite (iofwrite.c:45)
==5297==    by 0x4EC8A35: std::basic_ostream<char, std::char_traits<char> >& std::__ostream_insert<char, std::char_traits<char> >(std::basic_ostream<char, std::char_traits<char> >&, char const*, long) (streambuf:451)
==5297==    by 0x400D0F: main (in /tmp/1411705143.17426/a.out)
==5297==  Address 0x5c2a0a0 is 0 bytes inside a block of size 4 free'd
==5297==    at 0x4C2A09C: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5297==    by 0x400CD9: main (in /tmp/1411705143.17426/a.out)
==5297== 
==5297== Invalid read of size 1
==5297==    at 0x58E6DC7: _IO_default_xsputn (genops.c:479)
==5297==    by 0x58E4D19: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1393)
==5297==    by 0x58DA98C: fwrite (iofwrite.c:45)
==5297==    by 0x4EC8A35: std::basic_ostream<char, std::char_traits<char> >& std::__ostream_insert<char, std::char_traits<char> >(std::basic_ostream<char, std::char_traits<char> >&, char const*, long) (streambuf:451)
==5297==    by 0x400D0F: main (in /tmp/1411705143.17426/a.out)
==5297==  Address 0x5c2a0a2 is 2 bytes inside a block of size 4 free'd
==5297==    at 0x4C2A09C: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5297==    by 0x400CD9: main (in /tmp/1411705143.17426/a.out)
==5297==

I suspect that this solution doesn't work because the data inside the vector is pointing to garbage after the delete. I've also tried args.push_back(std::move(writable)). What is a workaround?

Upvotes: 0

Views: 316

Answers (3)

user3867376
user3867376

Reputation: 3

You are getting valgrind read issue because 'writable' is deleted before being read at line "cout << args[i] << "\n";".

You should delete it after referencing it.

Upvotes: 0

user4081979
user4081979

Reputation: 3

I decided to let a smart pointer handle my memory.

std::vector<std::unique_ptr<char[]>> args;

std::string str = "hey";

char * writable = new char[str.size() + 1];
std::copy(str.begin(), str.end(), writable);
writable[str.size()] = '\0'; // don't forget the terminating 0
args.push_back(std::unique_ptr<char[]>(writable));

Upvotes: 0

Srikanth
Srikanth

Reputation: 1003

The example says "Don't forget to free the string after using it". You're delete-ing the memory immediately after inserting the address into the vector. This leads to very broken code when you attempt to access the same addresses later.

Without going into the merits of your approach as a whole (and why you're doing what you're doing), the immediate fix for your problem is to invoke delete only after you are done with the use of the addresses in the vector.

Note that you need to loop over the vector and call delete on each address in the vector.

Upvotes: 2

Related Questions