C programming pointer arrays read file

Question

The line *array[cnt] = thing causes a seg fault and I don't know why. Any ideas to fix this?

long *Load_File(char *Filename, int *Size)
{
    FILE *fp;

    if((fp = fopen(Filename,"r")) == NULL)
    {
        printf("Cannot open file.\n");
        exit(-1);
    }

    fscanf(fp,"%d",Size);

    int cnt = 0;
    int items = *Size;
    long * array[items];
    int thing;

    while (!feof(fp))
    {
        fscanf(fp,"%d",&thing);
        *array[cnt] = thing;
        cnt++;
    }

    fclose(fp);

    return *array;
}

Answer 1

long * array[items];

is declaring an array of pointers to long data type. But these pointers are not pointing to anything meaningful.

When you do

*array[cnt] = thing;

you are dereferencing the pointer which is incorrect since they dont point to anything meaningful.

You can dynamically allocate the memory for the array as:

long * array = (long*) malloc(size(long) * items);

and then do:

while (!feof(fp)) {
        fscanf(fp,"%d",&arr[cnt++]);
    }

and then return the array as:

return array;

Answer 2

First off, that code can't compile. Since items isn't a constant, it can't be used to size the array. How did you get it to even run, let alone seg-fault? Aside from that, and in addition to the problem @codaddict highlights...

feof won't return true until fscanf fails. This will push you past the end of the array. Better to write this:

while (cnt < items && fscanf(fp, "%d", &thing))
{
    /* ... */
}

WRT the array, I think you intended this:

long *array = malloc(sizeof(long)*items);
/* ... */
    array[cnt] = thing;
/* ... */
return array;

Answer 3

Change

long * array[items];

to

long * array = (long *) malloc(sizeof(long) * items);

We dynamically allocate memory for items longs and store the address in our array variable. Your syntax means "An array of items pointers to long". The new syntax means "a pointer to a long" (the first of a dynamic "array").

Change

    *array[cnt] = thing;

to

    array[cnt] = thing;

We put the latest read long in the correct spot.

Change

return *array;

to

return array;

We return array, which is the same as a pointer to the first long in the memory we allocated. Be sure to free it later.

EDIT:

Thanks to ergosys for reminding me that VLAs are allocated on the stack. Removed suggested changes to function header.