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.

Hello, I stopped C some years ago, but this line : array = malloc(0 * sizeof(*array)); will always allocate PointerSize bytes. Your array is actually point on pointers, so *array is just a pointer. Edit, btw : 0*XX = 0, so you are not allocating any memory — Jurion
– Jurion, Commented Nov 2, 2014 at 3:11
Read This Question stackoverflow.com/questions/2937409/resizing-an-array-with-c ! — Blackhat002
– Blackhat002, Commented Nov 2, 2014 at 3:22
@Blackhat002 Hi, I have looked at that question quite extensively. I probably missed an important detail though. I've tried all sorts of ways, but they either have an invalid pointer or segmentation fault. — Ansdai
– Ansdai, Commented Nov 2, 2014 at 3:25
@Jurion malloc(0) may return NULL. "... If the size of the space requested is zero, the behavior is implementation-defined: either a null pointer is returned, or the behavior is as if the size were some nonzero value ..." C11 §7.22.3 1 Agree the code could simply array = 0;. — chux
– chux, Commented Nov 2, 2014 at 3:49
1) remove the malloc of 0 bytes line from the program. 2) the current code is only changing the passed in parameter, not the underlying variable back in the callers code. suggest changing all: 'array = realloc(array,' to '*array = realloc( *array,' — user3629249
– user3629249, Commented Nov 2, 2014 at 11:30

Barmar · Accepted Answer · 2014-11-02 03:25:46Z

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");
        }

    }
}

Hello, thank you and Jonathan, I understand this better now. However, this is one problem with the implementation above, when the input hits end of file, a segmentation fault will occur for some reason.
I'm not sure why. When it gets to the end of file, fgets() should return NULL and the while loop ends. I guess you need to run it in a debugger to see where how it's getting to the line that causes the segfault.
I figured it out. It was because instead of curSize in malloc, I accidentally put a 0. But I can't see why that would cause fgets to seg fault.
If you write outside the bounds of an array, anything can happen. Including some other, unrelated function getting an error because you've corrupted the heap.

Jonathan Leffler · Accepted Answer · 2014-11-02 03:31:42Z

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 **.

this code: data[n_used++] will not properly step to the next entry in the memory pointed to by data, unless it happens that int and long int are the same length. This may be true, but is a poor assumption. much better to change int *data to long int *data.
@user3629249: Can you explain why long is relevant when the code in the question has no use of long?

Collectives™ on Stack Overflow

How to resize array of integers?

2 Answers 2

4 Comments

2 Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

2 Comments

Linked

Related