c++stringpointersstreamostringstream

C++ ostringstream strange behavior


I had a very strange problem with c++ code recently. I have reproduced the case in minimalist example. We have an Egg class:

class Egg
{
private:
    const char* name;
public:
    Egg() {};
    Egg(const char* name) {
        this->name=name;
    }
    const char* getName() {
        return name;
    }
};

We also have a Basket class to hold the Eggs

const int size = 15;
class Basket
{
private:
    int currentSize=0;
    Egg* eggs;
public:
    Basket(){
        eggs=new Egg[size];
    }
    void addEgg(Egg e){
        eggs[currentSize]=e;
        currentSize++;
    }
    void printEggs(){
        for(int i=0; i<currentSize; i++)
        {
            cout<<eggs[i].getName()<<endl;
        }
    }
    ~Basket(){
        delete[] eggs;
    }
};

So here is example that works as expected.

 Basket basket;
 Egg egg1("Egg1");
 Egg egg2("Egg2");

 basket.addEgg(egg1);
 basket.addEgg(egg2);
 basket.printEggs();
 //Output: Egg1 Egg2

This is the expected result, but if I want to add N eggs with generated names depending on some loop variable, I have the following problem.

 Basket basket;
 for(int i = 0; i<2; i++) {
    ostringstream os;
    os<<"Egg"<<i;
    Egg egg(os.str().c_str());
    basket.addEgg(egg);
 }
 basket.printEggs();
 //Output: Egg1 Egg1

If I change the loop condition to i<5, I get "Egg4 Egg4 Egg4 Egg4 Egg4". It saves the last added Egg in all the indexes of the dynamic Egg array.

After some search in google I found out that giving the char* name variable in Egg a fixed size and using strcpy in the constructor fixes the issue.

Here is the "fixed" Egg class.

class Egg
{
private:
     char name[50];
public:
    Egg(){};
    Egg(const char* name)
    {
        strcpy(this->name, name);
    }
    const char* getName()
    {
        return name;
    }
};

Now the question is why?

Thanks in advance.

Here is a link to the whole code.


Solution

  • Lets take a closer look at this expression: os.str().c_str().

    The function str returns a string by value, and using it this way make the returned string a temporary object whose lifetime is only until the end of the expression. Once the expression ends, the string object is destructed and exists no more.

    The pointer you pass to the constructor is a pointer to the internal string of the temporary string object. Once the string object is destructed that pointer is no longer valid, and using it will lead to undefined behavior.

    The simple solution is of course to use std::string whenever you want to use a string. The more complicated solution is to use an array and copy the contents of the string, before it disappears (like you do in the "fixed" Egg class). But be aware of that the "fixed" solution using fixed-sized arrays is prone to buffer overflows.