0

I have written a function that concatenates strings by using HeapAlloc() and HeapFree() and I want to check the return of these functions. However, if allocation fails, I must try again to allocate until it works. How to do that ?

I link this question with this one.

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

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

    // Do the copy and concat
    if (*buffer == NULL)
    {
        *buffer = (LPTSTR)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, newSize * sizeof(*buffer));
        _tcsncpy_s(*buffer, newSize, msg, _tcsclen(msg));
    }
    else
    {
        *buffer = (LPTSTR)HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, *buffer, newSize * sizeof(*buffer));
        _tcsncat_s(*buffer, newSize, msg, _tcsclen(msg));
    }
}
Community
  • 1
  • 1
  • 1
    My comments: 1. No error checking at all. 2. Use of `HeapAlloc` makes it non-portable. 3. Wasteful to call to `tcslen()` multiple times with same arguments. 4. No need to use `_tcsncpy_s` or `_tcsncat_s` in the illusion of security. Since you worked out the lengths once, use `memcpy`. 5. Don't use `TCHAR` unless you plan to support Windows 9x. – David Heffernan Jun 17 '14 at 13:58
  • I don't want my code be portable. It must support Windows 9x. Why "illusion of security" ? I don't know what to do if allocations fail, this is also my question. – user3748455 Jun 17 '14 at 14:02
  • 1
    Why is `HeapAlloc` better than `malloc`? It makes no sense at all for me to do that. Your original post stated that you had made the code secure, I presume through the use of `_tcsncpy_s` and `_tcsncat_s`. But they don't add any security in this setting. You calculated buffer lengths with calls to `_tcsclen` after all. Just use `memcpy`. – David Heffernan Jun 17 '14 at 14:04
  • Also, `size` is not the best variable name. I'd use `length`. To me `size` implies a count of `char` but you use `size` for length of an array. – David Heffernan Jun 17 '14 at 14:05

1 Answers1

6

However, if allocation fail, I must try again to allocate until it works. How to do that?

No, if an allocation fails, it's all over. At that point you should terminate the process. There's no reason at all to believe that a subsequent allocation will succeed. Trying again and again won't help, computers don't work like that. And once you have reached low-memory state, you can expect problems left, right and centre. The safest course of action is to abort.

Now, it is possible to write programs that can survive low-memory states. But this requires the memory allocation to be designed and handled very carefully. And it requires all parts of the program to adhere to that design. Your program is not of that nature.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490