0

So, i have a lib file where i have function which converts string to char* :

void Additional::str2Char(string s,char** cstr)
{
    *cstr = new char[s.length() + 1];
    *cstr = (char*) s.c_str();
}

Then, i create console app, and do this:

int main()
{
    Additional *a = new Additional();
    string b = "fdfd";
    char *test;
    a->str2Char(b, &test);
    cout << test << endl;
    delete a;
}

The ouput is really bad.. Help me, i dont know how to get the pointer from lib right.

molbdnilo
  • 55,783
  • 3
  • 31
  • 71
mrglut
  • 27
  • 5
  • You are referencing the data of a local string `s`. That reference becomes invalid outside of the function. Also, why are you dynamically allocating `a`? – juanchopanza Jan 26 '15 at 10:21
  • @juanchopanza i pass the string to this funciton not vice versa. Dont worry about "a", i think its not a problem. If a local - i have same problem. – mrglut Jan 26 '15 at 10:28
  • and how it is related to `C`? – Sourav Ghosh Jan 26 '15 at 10:29
  • `s` is a local variable in the function. On top of that, you never copy its data to the buffer you allocate. So you have a dangling pointer *and* a memory leak. – juanchopanza Jan 26 '15 at 10:29
  • 1
    @mrglut - Even though you say "don't worry about 'a'", it gives an insight on how you're approaching your issue. Perception means a lot... – PaulMcKenzie Jan 26 '15 at 10:32

3 Answers3

4

First you allocate some memory.

Then you reassign the pointer to that memory, to point to the contents of the local string s, leaking the memory you allocated.

Then you return from the function, destroying s, leaving the pointer dangling. It no longer points to valid data, and dereferencing it gives undefined behaviour.

The best solution is to stop messing around with pointers and new, and use std::string for all your strings. If you really want to do this, then you'll need to copy the string contents to the new memory:

*cstr = new char[s.length() + 1]; // no change
std::strcpy(*cstr, s.c_str());    // copy the data, including the terminator
Mike Seymour
  • 235,407
  • 25
  • 414
  • 617
0

If the library is not going to modify the text then, the c_str() call is enough. No need to create an char pointer/array.

If it is going to modify the text: https://stackoverflow.com/a/13294114/47351

strncpy(tab2, tmp.c_str(), sizeof(tab2));
tab2[sizeof(tab2) - 1] = 0;
Community
  • 1
  • 1
RvdK
  • 18,577
  • 3
  • 56
  • 100
0

Since you want to handle the memory allocation inside the function, a different API could be used:

// Creates a const char* and copies the contents of s to it. Caller
// takes ownership of the returned data.
const char* str2cstr(const string& s) {
    char* cstr = new char[s.size()];
    strncopy(cstr, s.c_str(), s.size());
    return cstr;
}

Now you can do const char* cstr = str2cstr(str);. Notice also that I'm using a const string& param to avoid the extra string copy.

iccananea
  • 171
  • 4