C++ char array corruption?

Question

I'm trying to build a function that compacts the sprintf function, but somehow I ran into the following problem:

The first line after calling the class (It used to be a function, but that didn't work either) I get the correct result: http://puu.sh/1m1Bw

But the line after I get something completely different, while I didn't even touch the class or the variable: http://puu.sh/1m1BR

Can someone explain to me what's happening here?

Edit: Forgot the actual class:

StringCreator::StringCreator(char* _parten, ...) {
        char buff[255];

        va_list args;
        va_start (args, _parten);
        vsprintf (buff,_parten, args);
        va_end(args);

        this->str = buff;
    }

And in the .h file:

class StringCreator {
public:
    StringCreator(char* _parten, ...);
    char* str;
};

No. It's impossible to explain what is happening unless you show us the code... — Jasper, Nov 03 '12 at 19:42
Oow shoot, fotgot the class: StringCreator::StringCreator(char* _parten, ...) { char buff[255]; va_list args; va_start (args, _parten); vsprintf (buff,_parten, args); va_end(args); this->str = buff; } — Thijs, Nov 03 '12 at 19:47

score 2 · Accepted Answer · edited May 23 '17 at 11:55

After the StringCreator() constructor is complete the member variable this->str is a dangling pointer as it is pointing to buff which is a local variable of the constructor. Accessing this->str after the constructor is undefined behaviour. Changing str from a char* to a std::string is a solution and if you need access to a const char* you can use str.c_str(). Using a std::string also means the default copy constructor and assignment operator are correct.

If, as indicated in the comment to this answer, you new char[255] instead of using a std::string, then you need to either make StringCreator non-copyable or implement a copy constructor and assignment operator that copies the content of str. See What is The Rule of Three? Alternatively, you could have char str[255]; and avoid dynamic allocation and default copying of StringCreator would be correct.

To avoid potential buffer overrun on the call to vsprintf() use vsnprintf() instead (if your compiler supports C99) which accepts as an argument the size of the buffer being populated and does not write more than the specified size.

Ofcourse! Thank you! Changed it to: `code` StringCreator::StringCreator(char* _parten, ...) { str = new char[255]; va_list args; va_start (args, _parten); vsprintf (str,_parten, args); va_end(args); } — Thijs, Nov 03 '12 at 19:55

C++ char array corruption?

1 Answers1