0

How do I do this thing.

char* ToString(int num) {

    char* str = new char[len(num)];

    //conversion

    return str;

}

And by calling this.

string someStr = ToString(someInt);

Should I free the someStr here?

I know I always need to delete whenever I use new.

And what if I call this function multiple times, do I allocate memory and just leaving them behind not using it?

Mike
  • 47,263
  • 29
  • 113
  • 177
mr5
  • 548
  • 1
  • 5
  • 14
  • why do I always get negative reputation. Im just asking – mr5 Feb 05 '13 at 12:20
  • I didn't down-vote, but one thing that springs to mind is that it isn't clear what you are doing inside the `ToString` function. Also, what is `len(int)`? – juanchopanza Feb 05 '13 at 12:46
  • We can just guess - you are using `char*` instead of string, `len(num)` seems mysterious, and you leak the memory. And if your compiler has some C++11 support, there already is `std::to_string` which does this correctly. – Bo Persson Feb 05 '13 at 12:48
  • ToString() is very obvious to explain while len() returns number of digits of an int – mr5 Feb 05 '13 at 12:53

5 Answers5

3

You should avoid this practice altogether. Either return a std::unique_ptr, or deal with std::string directly. It is not clear from your code what exactly you are trying to do, so I can't offer specific solutions.

Note that this initialization:

string someStr = ToString(someInt);

will only work properly if you return a null-terminated string, but it leaks resources regardless.

See this related post.

Community
  • 1
  • 1
juanchopanza
  • 223,364
  • 34
  • 402
  • 480
1

You need to call delete once for every call to ToString. You also can't initialise a std::string with an allocated char array in the way your question hints at - that'd leak the returned memory, with your someStr variable having copied it.

The easiest/neatest thing to do would be to change ToString to return std::string instead. In this case, memory used by the string will be automatically deleted when the caller's variable goes out of scope.

simonc
  • 41,632
  • 12
  • 85
  • 103
1

I ran your code under valgrind with --leak-check=full, it reports num size of memory leak.

Call new/delete, new [] /delete [] in pair is the only way to keep memory cycled.

I am not sure what's you trying to do, if you want to convert integer types to string, C++ has a few options:

// std::to_string(C++11) e.g:
{
    std::string str = std::to_string(num)
}
// std::stringstream     e.g:
{
    std::string str;
    std::stringstream ss;
    ss << num;
    ss >> str;
}
// boost::lexical_cast   e.g:
{
    std::string str = boost::lexical_cast<std::string>(num);
}
// itoa(c function)
{
    char buf[MAX_INT_DIGITS]; // MAX_INT_DIGITS == 12 ("-2147483648\0")
    itoa(num, buf, 10);
    std::string str(buf);
}
cristicbz
  • 421
  • 3
  • 7
billz
  • 44,644
  • 9
  • 83
  • 100
  • I derived my ToString() function from itoa() but it needs a stack memory, and I don't want do allocate much or less memory. – mr5 Feb 05 '13 at 12:29
  • I try deleting it inside and outside the scope but gets me an rt error. `char* str = ToString(2);` `delete [] str;` and this `char* temp = str;` `delete [] str;` `return temp;` – mr5 Feb 05 '13 at 12:48
  • no, you delete it then you can't return invalid memory out. and you can not derive your function from iota – billz Feb 05 '13 at 12:59
  • I mean by derive is I get the code from there and put in mine. why is it get invalidated? also deleting first the str(on heap) gets me an rt error? – mr5 Feb 05 '13 at 13:11
  • you could read qPCR4vir's answer, you need a good book to start with! – billz Feb 05 '13 at 13:24
0

You're ToString function should return a std::string, if you then just assign the value to a std::string. No reason to deal with dynamically allocated memory here.

Marius Bancila
  • 16,053
  • 9
  • 49
  • 91
0

someStr is a copy. You have allready a leak. You need to temporally save the value of the returned pointer and after constructing the string delect it. This is normaly the job of the smart pointers.

EDIT: No,

char* temp =  strs; delete [] str; return temp;

will something undefined. But:

char* temp =ToString(someInt);  string someStr(temp);delete []temp;

will work. But this is only for you to understand the idea. This can be made for you if you return a unique_ptr. And I'm assuming this is a kind of general question of returning a memory that have to be free after that, in with case unique_ptr and shared_ptr are a solution. In this particular case you can just create a string, modify it and return it simply. All the memory manage will be made for you by the string class. If you really only need to allocate “space” in the string, you can do:

String Str; Str.reserve(len(num));
qPCR4vir
  • 3,521
  • 1
  • 22
  • 32