Error when reallocating dynamic unsigned char array with invalid next size error

user_13857973 0
Viewed 354

I'm attempting to create a dynamic unsigned char array that grows in number of rows per each element added:

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

#define BLOCKSIZE 1

int main()
{
    unsigned char **p = NULL, rows = 0;
    // initialize p pointer to as the size of one char
    p = malloc ( BLOCKSIZE * sizeof ( unsigned char *) );
    // add 10 "TEST" elements
    for (;rows < 10; rows++ )
    {
        if (rows > BLOCKSIZE) {
            // allocate memory for each row
            p = realloc (p, rows * sizeof ( unsigned char *) );
        }
        // allocate memory for each column
        p[rows] = malloc (rows * sizeof (unsigned char  ));
        // add element
        p[rows] = "TEST";
    }
    for (int i=0; i < rows; i++)
        printf("%s\n", p[i]);

    return 0;
}

The code outputs this error when run:

realloc(): invalid next size
Aborted (core dumped)

Could someone please explain to me what I am doing wrong?

realloc malloc c arrays
2 Answers
  • In the realloc call:
p = realloc (p, rows * sizeof ( unsigned char *) );

In the first iteration of the cycle rows is 0, allocating 0 bytes makes it impossible for the first row to hold the assignment, and in subsequent reallocations there is not enough space for the number of pointers you are assigning, there is always one less than needed, this results in invalid access to unallocated memory.

You should at least reallocate with the size of 1 unsigned char*, you can also use the pointer itself (sizeof *p) in the block size:

p = realloc (p, (rows + BLOCKSIZE) * sizeof *p);
  • The second malloc does not reserve enough space, it should have at least the size of string you assign to it, plus the space for the null-terminator so 4 + 1.
p[rows] = malloc (5 * sizeof **p);

  • rows should be at the very least of int type, and ideally size_t, it's used to index the array there is no reason for it to be unsigned char.

  • As it is, the assignment of "TEST" is causing a memory leak, you are simply making p[rows] point to a string literal rather than to the memory block previously allocated. You should use:

memcpy(p[rows], "TEST", 5);

Or strcpy for signed char.

Fixed code(Online):

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

#define BLOCKSIZE 1

int main()
{
    unsigned char **p = NULL;
    size_t rows = 0;
    p = malloc ( BLOCKSIZE * sizeof *p);
    if(p == NULL){        //always check for allocation failure
        return EXIT_FAILURE;
    }
    for (;rows < 10; rows++ )
    {
        if (rows > BLOCKSIZE) {
            p = realloc (p, (rows + BLOCKSIZE) * sizeof *p );
        }
        if(p == NULL){
            return EXIT_FAILURE;
        }
        //in this case the row is always 4 + 1 chars, but you can have different number of chars per row
        p[rows] = malloc (5 * sizeof **p);
        if(p[rows] == NULL){
            return EXIT_FAILURE;
        }    
        memcpy(p[rows], "TEST", 5);
    }
    for (size_t i=0; i < rows; i++)
        printf("%s\n", p[i]);
    return EXIT_SUCCESS;
}
user_68659320

you have some bugs and i will try to explaing them and fix the code:

unsigned char **p = NULL, rows = 0;

rows is unsigned char type. why? it needs to of type size_t (becuase later we will use it in the maloc/realoc, and better to be of size_t.

for (;rows < 10; rows++ ){
...

so , i asume that you have typo and you have tried to iterate over integer/size_t data type not unsigned char!

void *realloc(void *ptr, size_t size);

The realloc() function shall deallocate the old object pointed to by ptr and return a pointer to a new object that has the size specified by size. The contents of the new object shall be the same as that of the old object prior to deallocation, up to the lesser of the new and old sizes. Any bytes in the new object beyond the size of the old object have indeterminate values. Upon successful completion, realloc() shall return a pointer to the (possibly moved) allocated space. If size is 0, either: A null pointer shall be returned and errno set to an implementation-defined value.

Any way assume that rows is of type size_t (inside the for loop), you need to reallocate like this: so, how to use realloc ?

unsigned char **p_temp = p;
p_temp = (unsigned char**)realloc(p_temp, (rows + BLOCKSIZE)*sizeof(unsigned char*));
if(p_temp ==NULL){
/*realloc failed but we still have p ! we didnt encounter a memory leak like your code*/
....
}
else{
p = p_temp; /*let p to point to the new allocated buffer */
}

p[rows] = malloc (rows * sizeof (unsigned char  ));

the same problems here!

Any way i didnt understand the logic in your code ? why not to allocate enough space and omit the call to realloc()?

user_87484720
Related