Combining std::strings and C-Strings causes buffer overrun

Question

I am attempting to make a function that will make combining C Strings and std::strings a little easier in my C++ WinAPI Application.

So instead of doing this:

TCHAR res[MAX_PATH];
_stprintf(res, _T("In functionX(): error occured where the variable values are %d, %u, %s, %c"), myInt, myUnsignedInt, myStr.c_str(), myChar);
MessageBox(NULL, res, _T("Error Occurred"), MB_OK);

I just have to do this(which makes it a little easier to merge different string types because I don't have to keep declaring TCHAR arrays everywhere):

tstring res = concat(_T("In functionX(): error occured with the variable values %d, %u, %s, %c"), myInt, myUnsignedInt, myStr.c_str(), myChar);
MessageBox(NULL, (LPTSTR)res.c_str(), _T("Error Occurred"), MB_OK);

My Problem: My function concat(); fails when I pass more than 1 variable inside the parameter format and I dont know why?

// The following function call causes the error
tstring ou = concat(_T("In functionX(): Failed to create temp file - %s - %s\r\n"), (LPTSTR)tempFileRootDir.c_str(), tempFile); 

tstring WinFile::concat( TCHAR* strFormat, TCHAR* format, ... )
{
    // tstring is either a std::string or std::wstring depending on whether unicode is used
    // Post: Wrapper function to easily merge C++ strings with C Strings

    va_list arguments;
    va_start(arguments, format);
    TCHAR res[MAX_PATH];
    _stprintf(res, strFormat, format); 
    return tstring(res);
}

The error that occurs when I run the function in Microsoft Visual C++ is:

A buffer overrun has occurred in Application.exe which has corrupted the program's internal state. Press Break to debug the program or Continue to terminate the program.

For more details please see Help topic 'How to debug Buffer Overrun Issues'.

Why are you declaring res to have MAX_PATH characters? You aren't copying a path to it. At a guess, I'd say you're trying to write more than MAX_PATH characters to res. — Gort the Robot, May 13 '12 at 05:13
As an option you can use ATL::CString. It has Format method. — Sergey Podobry, May 13 '12 at 06:44
Any particular reason for uppercasing `STD`? That usually means sexually transmitted disease, doesn't it? The C++ standard library namespace is `std`, in lower-case. — jalf, May 13 '12 at 06:49

Managu · Accepted Answer · 2012-05-13T06:41:49.310

1

It looks like your varargs handling is a bit off. And yes, by writing to a fixed size buffer, you're asking for a stack overflow.

To correct the first, you'll need to use a varargs-accepting version of printf. For the second, you should count before you print:

tstring WinFile::concat( TCHAR const * strFormat, ... )
{
    va_list args;

    // Determine how much space to reserve.
    va_start(args, strFormat);
    size_t msg_len=_vsctprintf(strFormat, args);
    va_end(args);

    // Reserve space on heap.
    //
    // "_vscprintf returns the number of characters that would be generated
    // if the string pointed to by the list of arguments was printed ... 
    // [and] does not include the terminating null character."
    //
    // So we add space for a terminating null.
    std::vector<TCHAR> writebuffer(1+msg_len);

    // perform formatting
    va_start(args, strFormat);
    _vstprintf(&writebuffer[0], strFormat, args);
    va_end(args);

    // return a copy as a tstring.
    return tstring(&writebuffer[0], &writebuffer[msg_len]);
}

edited May 13 '12 at 06:41

answered May 13 '12 at 05:29

Managu

8,849
2
30
36

1

I think `concat` as a name is a bit misleading. Perhaps `WinFile::format_msg`? – Managu May 13 '12 at 05:45
thanks for the teaching. I get an error declaring writebuffer: "Expression must have a constant value". Is my only solution to declare a vector or use a tstring instead? – sazr May 13 '12 at 06:28
Are dynamically sized arrays not standard C++ yet? Probably best to just use a `std::vector` then. In particular, trying to write directly into a `tstring` with a c-style function (`_vstprintf`) is almost certainly a bad idea. I've modified my answer accordingly. – Managu May 13 '12 at 06:46

Combining std::strings and C-Strings causes buffer overrun

1 Answers1