Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Concatenating a stack string with a heap string gives odd results

I am sure the following has a rational explanation but I am nevertheless a bit baffled.

The issue is with a function which creates a _TCHAR[CONSTANT], a _TCHAR*, concatenates them and returns the result.

For some reason the call to whatTheHeck() from _tmain() returns gibberish.

_TCHAR* whatTheHeck(_TCHAR* name) {
    _TCHAR Buffer[BUFSIZE];
    DWORD dwRet;
    dwRet = GetCurrentDirectory(BUFSIZE, Buffer);
    _TCHAR* what = new _TCHAR[BUFSIZE];
    what = _tcscat(Buffer, TEXT("\\"));
    what = _tcscat(what, name);
    return what;
}

int _tmain(int argc, _TCHAR* argv[]) {

    _TCHAR* failure = whatTheHeck(TEXT("gibberish);")); // not again ..
    _tprintf(TEXT("|--> %s\n"), failure);

    _TCHAR* success = createFileName(TEXT("readme.txt")); // much better
    _tprintf(TEXT("|--> %s\n"), success);

    return 0;
}

In contrast, when going with heap things work as expected.

_TCHAR* createFileName(_TCHAR* name) {
    _TCHAR* Buffer = new _TCHAR[BUFSIZE];
    DWORD dwRet;
    dwRet = GetCurrentDirectory(BUFSIZE, Buffer);
    Buffer = _tcscat(Buffer, TEXT("\\"));
    Buffer = _tcscat(Buffer, name);
    return Buffer;
}

Why the difference?

Is it because _tcscat() concatenates memory addresses instead of their contents and return purges the stack?

like image 358
Saul Avatar asked Jul 29 '11 10:07

Saul


2 Answers

There are lots of problems with your code. Let's take it apart, shall we:

_TCHAR* whatTheHeck(_TCHAR* name)   // We're inside a local scope
{
    _TCHAR Buffer[BUFSIZE];         // "Buffer" has automatic storage

    _TCHAR* what = new _TCHAR[BUFSIZE];  // "what" points to newly allocated dyn. memory

    what = _tcscat(Buffer, TEXT("\\"));  // Oh no, we overwrite "what" - leak!
                                         // Now what == Buffer.

    what = _tcscat(what, name);  // Equivalent to "_tcscat(Buffer, name)"

    return Buffer;               // WTPF? We're returning a local automatic!
 }

As you can see, you are both causing a memory leak with a gratuitious and reckless new, and you are also returning the address of a local object past its lifetime!

I would strongly recommmend

  1. reading the documentation for strcat and understanding "source" and "destination",
  2. not using strcat, but a safer version like strncat,
  3. not using strncat, but instead std::string.
like image 132
Kerrek SB Avatar answered Nov 16 '22 20:11

Kerrek SB


That happens because _tcscat concatenates into the destination parameter, which is the first one, and then returns it. So, it returns a pointer to the array Buffer and it gets stored in what in this line:

what = _tcscat(Buffer, TEXT("\\"));

Then this pointer is returned, and you have undefined behavior once you try to use it, because the local Buffer no longer exists.

Furthermore, the line above also causes the memory allocated for what to be leaked:

_TCHAR* what = new _TCHAR[BUFSIZE];
what = _tcscat(Buffer, TEXT("\\")); // this loses the memory allocated
                                    // in the previous line
like image 44
R. Martinho Fernandes Avatar answered Nov 16 '22 22:11

R. Martinho Fernandes