13

What is the best way to zero out new memory after calling realloc while keeping the initially allocated memory intact?

#include <stdlib.h>
#include <assert.h>
#include <string.h>
#include <stdio.h>

size_t COLORCOUNT = 4;

typedef struct rgb_t {
    int r;
    int g;
    int b;
} rgb_t;

rgb_t** colors;

void addColor(size_t i, int r, int g, int b) {
    rgb_t* color;
    if (i >= COLORCOUNT) {
        // new memory wont be NULL
        colors = realloc(colors, sizeof(rgb_t*) * i);
       //something messy like this...
        //memset(colors[COLORCOUNT-1],0 ,sizeof(rgb_t*) * (i - COLORCOUNT - 1));

         // ...or just do this (EDIT)
        for (j=COLORCOUNT; j<i; j++) {
            colors[j] = NULL;
        }

        COLORCOUNT = i;
    }

    color = malloc(sizeof(rgb_t));
    color->r = r;
    color->g = g;
    color->b = b;

    colors[i] = color;
}

void freeColors() {
    size_t i;
    for (i=0; i<COLORCOUNT; i++) {
        printf("%x\n", colors[i]);
        // can't do this if memory isn't NULL
       // if (colors[i])
         //   free(colors[i]);

    }
}


int main() {
    colors = malloc(sizeof(rgb_t*) * COLORCOUNT);
    memset(colors,0,sizeof(rgb_t*) * COLORCOUNT);
    addColor(0, 255, 0, 0);
    addColor(3, 255, 255, 0);
    addColor(7, 0, 255, 0);


    freeColors();
    getchar();
}
Nick Van Brunt
  • 15,244
  • 11
  • 66
  • 92
  • This has *very* poor performance when one color gets added to the end of the list of colors, the usual calling pattern. You'll only add one element at a time before reallocating. Consider at least allocating max(i+1, COLORCOUNT * 2). – Hans Passant Jan 26 '10 at 18:07
  • 1
    This is just an example to illustrate the problem. The actual source is a hash table which resizes by prime number IIRC – Nick Van Brunt Jan 26 '10 at 18:12

5 Answers5

9

There is no way to solve this as a general pattern. The reason why is that in order to know what part of the buffer is new you need to know how long the old buffer was. It's not possible to determine this in C and hence prevents a general solution.

However you could write a wrapper like so

void* realloc_zero(void* pBuffer, size_t oldSize, size_t newSize) {
  void* pNew = realloc(pBuffer, newSize);
  if ( newSize > oldSize && pNew ) {
    size_t diff = newSize - oldSize;
    void* pStart = ((char*)pNew) + oldSize;
    memset(pStart, 0, diff);
  }
  return pNew;
}
JaredPar
  • 733,204
  • 149
  • 1,241
  • 1,454
5

There is probably no need to do the memset: you may not be using colors[k] before setting it with something valid later. For example, your code sets colors[i] to a newly allocated color pointer, so you didn't need to set colors[i] to NULL.

But, even if you wanted to "zero it out so everything is nice", or really need the new pointers to be NULL: the C standard doesn't guarantee that all-bits-zero is the null pointer constant (i.e., NULL), so memset() isn't the right solution anyway.

The only portable thing you can do is to set each pointer to NULL in a loop:

size_t k;
for (k=COLORCOUNT; k < i+1; ++k) /* see below for why i+1 */
    colors[k] = NULL;

Your primary problem is that your realloc() call is wrong. realloc() returns a pointer to the resized memory, it doesn't (necessarily) resize it in-place.

So, you should do:

/* i+1 because you later assign to colors[i] */
rgb_t **tmp = realloc(colors, (i+1) * sizeof *tmp);
if (tmp != NULL) {
    /* realloc succeeded, can't use colors anymore */
    colors = tmp;
} else {
    /* realloc failed, colors is still valid */
}

If you really want to know what the memset() call should be, you need to set to zero the memory starting at colors+COLORCOUNT, and set i+1-COLORCOUNT members to zero:

memset(colors+COLORCOUNT, 0, (i+1-COLORCOUNT) * sizeof *colors);

But as I said above, all bytes zero is not guaranteed to be a NULL pointer, so your memset() is useless anyway. You have to use a loop if you want NULL pointers.

Alok Singhal
  • 93,253
  • 21
  • 125
  • 158
  • Thanks - the realloc error was an oversight while quickly writing up the test but I did not realize all bytes zero does not guarantee to NULL all of array members as per standard. – Nick Van Brunt Jan 26 '10 at 19:40
  • Saying that assuming NULL == 0 is "pointless" is a bit of an overstatement. Even if it's not explicitly required by the standard, it is the case in every known C compiler that I'm aware. Moreover, it can significantly speed up bug-squashing to use calloc or memset to force bad reads. I would rather enforce NULL == 0 via `assert(NULL == 0)` at the beginning of the program than write a ton of complex and bizarre code to support that one random implementation that chooses something else. – Dan Bechard Dec 26 '18 at 02:45
  • @DanBechard `NULL == 0` is always true. So, you would have to do your `assert` somewhat like this: `void * null_ptr = NULL; unsigned char * null_ptr_byte = &null_ptr; for (size_t i = 0; i < sizeof(null_ptr); ++i) assert(!null_ptr_byte[i]);` - oh, and you can't just do `memset(&null_ptr, 0, sizeof(null_ptr)); assert(!null_ptr);` because all bits zero doesn't need to be a valid pointer representation at all but might be a trap representation or otherwise cause undefined behavior, which you wanted to prevent in the first place - basicly rendering your assert useless. – Bodo Thiesen Oct 19 '20 at 08:44
  • @BodoThiesen If you could guarantee `NULL == 0` were always true, you wouldn't need to assert anything at all. That's the point! – Dan Bechard Oct 22 '20 at 06:19
  • @DanBechard "An integer constant expression with the value 0, or such an expression cast to type void *, is called a null pointer constant. 66) If a null pointer constant is converted to a pointer type, the resulting pointer, called a null pointer, is guaranteed to compare unequal to a pointer to any object or function." footnote: "66) The macro NULL is defined in (and other headers) as a null pointer constant; see 7.19." 6.3.2.3 (3) on page 55 of the ISO C standard (ISO/IEC 9899:2011) The problem that was discussed here was that the bit representation doesn't need to be zeros. – Bodo Thiesen Oct 22 '20 at 17:47
4

First of all, realloc can fail, so you need to check for NULL. Second, there is no better way to zero out the memory: just memset from the end of the old buffer to the end of the larger buffer.

florin
  • 13,986
  • 6
  • 46
  • 47
  • my commented out section crashes visual studio with "msvcr90d.dll!memset(unsigned char * dst=0x00000000, unsigned char value='', unsigned long count=2553828) Line 103 Asm". Am I just off by 1? – Nick Van Brunt Jan 26 '10 at 17:40
  • 1
    it looks like you're passing the value at colors[COLORCOUNT-1] to be reallocated, rather than a pointer to that location. – atk Jan 26 '10 at 17:42
  • realloc may not be able to reallocate in-place and will return a new pointer. You should check this and update color with the new value if it is not NULL. That's one reason the code may fail. The other reason may be that realloc couldn't allocate more memory (it returned NULL to tell you this, but you discard it). – Draemon Jan 26 '10 at 17:44
  • 1
    All bytes zero may or may not be a null-pointer constant. See my answer for details. – Alok Singhal Jan 26 '10 at 17:50
3

Write your own function, say reallocz, that accepts the current size as a parameter, and calls realloc and memset for you. It really won't get much better than what you already have... it's C, after all.

Thomas
  • 174,939
  • 50
  • 355
  • 478
2

On Linux, there is now size_t malloc_usable_size (void *ptr), provided by <malloc.h>. Find the man page at https://linux.die.net/man/3/malloc_usable_size.

This means you now can write a general-purpose wrapper for realloc(3):

void *realloc_zero(void *oldptr, size_t reqsize) {
  size_t oldsize = malloc_usable_size(ptr);
  void *newptr;
  if ((newptr = realloc(ptr, reqsize)) == NULL) {
    return NULL;
  }
  size_t newsize = malloc_usable_size(newptr);
  if (oldsize < newsize) {
    memset((char *) newptr + oldsize, 0, newsize - oldsize);
  }
  return newptr;
}

Note that malloc_usable_pointer(3) doesn't necessarily return the same number of bytes that you called calloc(3) with -- for example,

void *p = calloc(8, 1);
printf("%zu\n", malloc_usable_pointer(p));

prints 16 on my Linux machine. Also note that you should only call the above realloc_zero() on heap allocations obtained through calloc(3), since calloc() will zero-initialize any extra bytes reported by malloc_usable_pointer().