How to resize array of integers?

Question

I've been looking at other answers of similar nature here but still run into problems.

I am a beginner in C and need some advice.

Relevant parts of code:

int readNumbers(int **array, char* fname, int hexFlag) {

    int numberRead = 0;
    FILE* fp;
    int counter = 0;
    char arr[100];
    char* ptr;

    array = malloc(0 * sizeof(*array)); //Problematic area. Should I initially give it space?



    fp = fopen(fname, "r");  

    if (fp == NULL) {
            printf("Error opening file\n");
            return -1;
    }

    while (fgets(arr, sizeof(arr), fp)) {
            ptr = strtok(arr, " \n");
            while(ptr) {
               if (hexFlag == 0) { 
                        array = realloc(array, (counter + 1) * sizeof(int*)); 
                        array[counter++] = strtol(ptr , NULL , 10); //Seg Faulting
               } else {
                        array = realloc(array, (counter + 1) * sizeof(int*));
                        array[counter++] = strtol(ptr, NULL, 16);
               }
               ++numberRead;
               ptr = strtok(NULL , " \n");
    }

}

I debugged this and it seems that the array never gets memory allocated to it. Also, the program seg faults right after I try to access array by array[counter++];

I also found out it is bad practice to realloc after each increment, but I don't know what else to do.

Answer 1

The rule of thumb is to allocate a small initial allocation and double the allocated size when more space is necessary. You have to keep track of two sizes — the size allocated and the size in use. You can do it all with realloc(), though it is perhaps more conventional to use malloc() first and then realloc().

size_t n_alloc = 1;
size_t n_used = 0;
int *data = malloc(n_alloc * sizeof(*arr));

if (arr == 0)
    …report out of memory and return…
int base = (hexFlag == 0) ? 10 : 16;

while (fgets(arr, sizeof(arr), fp))
{
    ptr = strtok(arr, " \n");
    while (ptr)
    {
        if (n_used == n_alloc)
        {
            size_t new_size = n_alloc * 2;
            int *new_data = realloc(data, new_size);
            if (new_data == 0)
            {
                free(data);
                …report error and return…
            }
            data = new_data
            n_alloc = new_size;
        }
        data[n_used++] = strtol(ptr, NULL, base);
        ptr = strtok(NULL, " \n");
    }
}

/* Optionally resize array */
*array = realloc(data, n_used * sizeof(*data));
/* Or, instead of realloc(), just write: *array = data; */
return n_used;

Alternatively, the initialization could be:

size_t n_alloc = 0;
size_t n_used = 0;
int *data = 0;

This will also work fine; it even cuts down on the number of places where error reporting is required.

Note that the code carefully avoids leaking memory in case of realloc() failing. It is a mistake to write:

ptr = realloc(ptr, size);

If the realloc() fails, ptr is assigned NULL, which means you cannot release the memory that was allocated before the call to realloc(), which is an archetypal memory leak.

Also note that this code treats array (an int **) and data (an int *) correctly. The original code in the question was treating array as if it was an int *, not an int **.

Answer 2

It's bad practice to realloc every time through the loop. It's better to grow in larger blocks as needed.

Also, you're allocating the array incorrectly. array is a pointer to an integer array, you need to indirect through it to set the caller's pointer.

*array = malloc(size * sizeof(**array));

So your function should be like this:

int readNumbers(int **array, char* fname, int hexFlag) {

    int numberRead = 0;
    FILE* fp;
    int counter = 0;
    char arr[100];
    char* ptr;
    size_t curSize = 16;
    int radix = hexFlag ? 16 : 10;

    *array = malloc(curSize * sizeof(**array));

    fp = fopen(fname, "r");  

    if (fp == NULL) {
        printf("Error opening file\n");
        return -1;
    }

    while (fgets(arr, sizeof(arr), fp)) {
        ptr = strtok(arr, " \n");
        while(ptr) {
            if (counter >= curSize) {
                curSize += 16;
                *array = realloc(*array, curSize * sizeof(**array));
            }
            (*array)[counter++] = strtol(ptr , NULL , radix);
            ++numberRead;
            ptr = strtok(NULL , " \n");
        }

    }
}