2

I am trying to copy ostringstream to a char* array and am hoping someone can help me with understanding where my mistake lies. I looked on the forum, found some things that are similar and unfortunately am still am unable to get property copy from ostringstream to char*. In short I am trying to copy into the char* via:

ostringstream bld
bld<<"test"<<"is"<<"good"
const char * result = bld.str().c_str();

The complete code to reproduce an error is below. In this code I am having two functions that essentially build strings via ostringstream. In the makeFilePath() function I build a complete file path (ie /path/file.txt). In the add() function, I simply add two more char* arrays to an argument to get prefix /path/file.txt suffix.

The problem is for some unknown reason to me the fullFilePath also changes to look like prefix /path/file.txt suffix. Last three lines of a code will exhibit that.

I spent hours on this, thinking maybe this is a referencing issue or something else. However, none that I attemped worked. Any ideas how to get over this problem?

Thanks!!

#include <iostream>
#include <sstream>
#include <string>

using std::cout;
using std::endl;
using std::string;

using std::ostringstream;

const char* add(const char *fileName) {
    ostringstream bld;
    const char *prefix = "prefix";
    const char *suffix = "suffix";
    bld << prefix << " " << fileName << " " << suffix;
    string temp = bld.str();
    cout << "in add(): \n\t" << temp << endl;
    return temp.c_str();
}

const char * makeFilePath(const char *path, const char *name, const char *ext =
        ".txt") {
    ostringstream bld;
    bld << path << name << ext;
    cout << "makeFilePath(), returning: \n\t" << bld.str()<< endl;
    string temp = bld.str();
    return temp.c_str();

}

int main(int argc, char **argv) {
    cout << "=== PROJECT START ===" << endl;

    const char * filePath = "\\Path\\";
    const char *fileName = "FILENAME";
    const char *fullFilePath = makeFilePath(filePath, fileName);

    cout << fullFilePath before calling add():\n\t" << fullFilePath << endl;    
    const char* str = add(fullFilePath);
    cout << fullFilePath after calling add():\n\t" << fullFilePath << endl;

    return 1;
}
JavaFan
  • 1,295
  • 3
  • 19
  • 28
  • 2
    Is the result of `c_str()` still valid when the originating string is destroyed? I think it's invalidated at that point. You may need to copy it to a conventionally allocated C string. The question remains why you're mixing C and C++ here when a pure C++ approach wouldn't have issues like that. – tadman Apr 22 '15 at 15:47
  • Thanks for fast reply, #tadman. I thought about it as well and I believe I am copying the c_str() into the string temp. So while the c_str() is destroyed the copy should still be in the temp variable now, right? – JavaFan Apr 22 '15 at 15:51
  • 1
    `temp` is destroyed as soon as that function exits, so it's still not sufficient to persist that. `c_str()` just returns a `const char*` pointer to the contents of the string, it doesn't do anything more, so when that string is deleted, that pointer is no longer valid. – tadman Apr 22 '15 at 15:52
  • If you *really* need to return a const char* you could make `temp` static in both functions. There's one caveat though: Thread-safety. Calling either function from different threads simultaneously will get you into trouble. As long as you can avoid that you should be OK. – marcbf Mar 17 '20 at 06:42

3 Answers3

4

stringstream.str() returns a expiring value a temporary string object whose lifetime is restricted to the end of the expression. A pointer to a temporary expiring value when deleted would simply dangle and accessing it beyond the expression is an Undefined behaviour possibly a crash or some garbage value.

One option would be to fetch the string object to a persistent string and then get the constant pointer to the character array buffer.

const std::string bld= stringstream.str();
const char* result= bld.c_str();

Alternatively you may also consider to bind the expiring rvalue to a constant reference (Note some compilers might be lenient to bind to a reference which is possibly incorrect) and then fetch the pointer to the buffer. This will simply extend the lifetime of the temporary object.

But that would not resolve all you pain. As I have missed your function implementation

const char * makeFilePath(const char *path, const char *name, const char *ext =
        ".txt") {
    ostringstream bld;
    bld << path << name << ext;
    cout << "makeFilePath(), returning: \n\t" << bld.str()<< endl;
    string temp = bld.str();
    return temp.c_str();

}

You are returning a pointer to a local object the scope of which is not extended beyond the function scope. A better suggestion would be to return a string object from your function and then on the caller scope convert the string object to a const C null terminated string.

std::string makeFilePath(const char *path, const char *name, const char *ext =
        ".txt") {
    ostringstream bld;
    bld << path << name << ext;
    cout << "makeFilePath(), returning: \n\t" << bld.str()<< endl;
    return bld.str();

}

Alternatively you may want to duplicate the string using strdup, but the caller should be aware to release the resource for the allocated buffer.

Abhijit
  • 62,056
  • 18
  • 131
  • 204
  • Your answer is the approach taken in the question inside `makeFilePath`, but it's still incorrect. Your `bld` isn't really 'persistent' - it dies at the end of the function and therefore `return result;` isn't valid. – Aaron McDaid Apr 22 '15 at 15:53
  • @AaronMcDaid: I overlooked his implementation of makeFilePath – Abhijit Apr 22 '15 at 15:59
4

The short answer is you need to use something like strdup to fix this:

const char* makeFilePath(const char *path, const char *name, const char *ext = ".txt")
{
  ostringstream bld;
  bld << path << name << ext;
  cout << "makeFilePath(), returning: \n\t" << bld.str()<< endl;

  return strdup(bld.str().c_str());
}

This is a really sub-optimal solution as now you have a memory leak unless you properly free the result of this function, plus it might return NULL on occasion which could cause chaos if you don't test for it. It'd be much better to return std::string.

If you're using C++, use C++.

tadman
  • 208,517
  • 23
  • 234
  • 262
  • Thanks #tadman! Yes you make a good point if using c++ I should stick to c++. I thought that char type was native to C++ or is it a 'carry-over' from C? – JavaFan Apr 22 '15 at 16:57
  • C++ goes to great lengths to be inter-operable with C, so you can still use C-style strings if you want, but that's intended for situations where it can't be avoided, like interfacing with system libraries. Use C++ strings whenever you can, they're much easier to manage and don't expose you to buffer overflow problems and memory leaks are less of an issue if you use copies and references instead of pointers. – tadman Apr 22 '15 at 17:00
  • Thanks, for a clarification #tadman! I will use c++ strings in c++. I sort of had a hunch that chars in c++ were that a bad idea anyway, but I have this c++ textbook and they use char 90% of the time there. So I figured, hey why not try chars since I donot have to do #include and using std::string and it sort of felt that chars were more native to c++ than strings. – JavaFan Apr 22 '15 at 17:35
  • `std::string` behaves a lot more like Java's `String`, so you'll have a much better time using those. Manipulating C strings is way more work, especially since you're responsible for managing your own memory, cleaning up allocations, ensuring the strings are properly terminated, and that your buffers are the right size. In short, not fun. – tadman Apr 22 '15 at 17:39
2

Just don't use char *. If you're writing C++, you should use C++ strings, not C strings.

C doesn't have anything to copy strings for you, (except maybe sprintf with the NULL-target GNU extension?), so you are often going to be left with invalid pointers.

using std::ostringstream;
using std::string;

string add(const string fileName) {
    ostringstream bld;
    string prefix = "prefix";
    string suffix = "suffix";
    bld << prefix << " " << fileName << " " << suffix;
    string temp = bld.str();
    cout << "in add(): \n\t" << temp << endl;
    return temp;
}

string makeFilePath(string path, string name, string ext =
        ".txt") {
    ostringstream bld;
    bld << path << name << ext;
    cout << "makeFilePath(), returning: \n\t" << bld << endl;
    string temp = bld.str();
    return temp;

}

int main(int argc, char **argv) { // unfortunately, argv much be char**
    cout << "=== PROJECT START ===" << endl;

    string filePath = "\\Path\\";
    string fileName = "FILENAME";
    string fullFilePath = makeFilePath(filePath, fileName);

    cout << fullFilePath before calling add():\n\t" << fullFilePath << endl;    
    string str = add(fullFilePath);
    cout << fullFilePath after calling add():\n\t" << fullFilePath << endl;

    return 1;
}

There's a broader lesson here, in modern C++ you can delete almost all use of pointers (not just char pointers)

Aaron McDaid
  • 26,501
  • 9
  • 66
  • 88
  • Thanks, #Aaron McDald, I am not too keen on C++ vs C nuances and really helps to get to know them as I learn more c++ – JavaFan Apr 22 '15 at 17:26