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

OMG, This code has too many issues to be pointed out. Anyway, I will let you know the things you need to learn (immediately), (1) Scope of variables (2) Pointers and Arrays, — Alphaneo
– Alphaneo, Commented Feb 10, 2010 at 5:55

codaddict · Accepted Answer · 2010-02-10 04:23:46Z

3

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;

edited Feb 10, 2010 at 4:23

answered Feb 10, 2010 at 4:15

codaddict

457k83 gold badges501 silver badges537 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

codaddict Over a year ago

You need to make sure your file has a number "items" followed by at max those many numbers.

Marcelo Cantos · Accepted Answer · 2010-02-10 04:24:58Z

0

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;

answered Feb 10, 2010 at 4:24

Marcelo Cantos

187k40 gold badges338 silver badges366 bronze badges

3 Comments

Alok Singhal Over a year ago

While everything else you said is correct, even return array; is wrong because array is local to the function, so you can't return &array[0];.

Matthew Flaschen Over a year ago

You /can/ compile it using variable length arrays. However, as ergosys reminded us, you should never return a pointer to your stack (which includes VLAs). Alok, return array is not wrong. Since he malloced, array is pointing to heap memory, which is fine.

Marcelo Cantos Over a year ago

@Matthew, thanks for pointing that out. I'm a pre-C99 programmer.

Matthew Flaschen · Accepted Answer · 2010-02-10 04:52:25Z

0

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.

edited Feb 10, 2010 at 4:52

answered Feb 10, 2010 at 4:27

Matthew Flaschen

286k53 gold badges523 silver badges554 bronze badges

3 Comments

ergosys Over a year ago

All good except the last part! Returning a pointer to stack memory is bad juju.

pythonicate Over a year ago

the function header is predefined and im not allowed to change it

Scott Smith Over a year ago

What ergosys said, but it also looks like he's supposed to be returning the Size value that he reads to a variable provided by the caller (i.e. Size appears to be an [out] parameter). Filename is probably an [in], although it's not const, so who knows. The return value presumably is an array of longs, so I assume that the expectation was for the function to allocate the array on the heap and return the pointer.

Collectives™ on Stack Overflow

C programming pointer arrays read file

3 Answers 3

1 Comment

3 Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

3 Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related