Concat two strings in C and allocate dynamically buffer

Question

I know this kind of question has already been asked. I also used the solution of this topic to my tests. However, I want to know how using this kind of function without memory leak neither exception.

Note: LPTSTR ~ char* and LPCTSTR ~ const char*

void add_to_buffer(LPTSTR* buffer, LPCTSTR msg) {
    // Determine new size
    int newSize = 0;

    // Allocate new buffer
    if (*buffer == NULL)
        newSize =  _tcsclen(msg) + 1; // strlen()
    else
        newSize = _tcslen(*buffer) + _tcsclen(msg) + 1;

    LPTSTR newBuffer = (LPTSTR)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, newSize); // malloc()

    // Do the copy and concat
    if (*buffer == NULL)
        _tcscpy(newBuffer, msg); // strcpy()
    else
    {
        _tcscpy(newBuffer, *buffer);
        _tcscat(newBuffer, msg); // strcat()
        // release old buffer
        HeapFree(GetProcessHeap(), 0, *buffer); // free()
    }

    // store new pointer
    *buffer = newBuffer;
}

Tests:

LPTSTR test = NULL;
add_to_buffer(&test, _T("User:\r\n"));
add_to_buffer(&test, _T("42"));

First call to add_to_buffer works. However, the second function call causes an exception at HeapFree. I'm sure this is a problem about pointers, but I do not understand how to fix it.

Is it a good method? How to fix my exception?

score 0 · Answer 1 · answered Jun 17 '14 at 12:40

0

If your program is unicode-enabled, you're not allocating enough memory - because string length (in symbols) and string size (in bytes) don't match.

If it isn't, I don't see reason of using non-standard-C types over standard ones. It shouldn't be a problem though.

answered Jun 17 '14 at 12:40

keltar

17,711
2
37
42

I think I allocate enough memory (_tcsclen is a wide-character version of strlen if the program is unicode-enabled; _tcsclen and strlen behave identically otherwise). I use the WinAPI, my program is supported only by a Windows OS. I think the problem comes from the `free` function call. – user3748455 Jun 17 '14 at 12:47
You really don't get it. If unicode is enabled, your characters isn't one-byte-wide. Length and size becomes completely different terms (was the same with single-byte encoding). You using length as if it is size, - and it is not enough. strcpy then exceeds your buffer size, corrupting heap structure and much much later free cannot live with it. – keltar Jun 17 '14 at 12:56

alk · Accepted Answer · 2014-06-17T13:24:15.253

0

If you are compiling the code as multi-byte application this line

LPTSTR newBuffer = (LPTSTR)HeapAlloc(
  GetProcessHeap(), 
  HEAP_ZERO_MEMORY, 
  newSize
);

might allocate to few memory.

To fix this use:

LPTSTR newBuffer = (LPTSTR)HeapAlloc(
  GetProcessHeap(), 
  HEAP_ZERO_MEMORY, 
  newSize * sizeof(*newBuffer)
);

Besides this and the fact that the code lacks error checking for system calls the code looks fine.

However, to simplify things one can use HeapReAlloc() instead of the combination of HeapAlloc() and HeapFree().

If the program crashes anyway this might due to the memory management already being mashed up before this actual crash you observe.

edited Jun 17 '14 at 13:24

answered Jun 17 '14 at 12:44

alk

69,737
10
105
255

Ok, I get it. Could you give me an example with `HeapReAlloc()` in my function `add_to_buffer` ? – user3748455 Jun 17 '14 at 13:03
@user3748455: I'm looking forward to comment on your proposal how you'd do it, that you'd post in another question, which you are free to link from this question. Ok? ;-) – alk Jun 17 '14 at 13:07

Concat two strings in C and allocate dynamically buffer

2 Answers2

Linked