How to read values from an unspecified file size and store them dynamically in a vector in C?

Question

Said I have anumbers.txt file with an unspecified size of double numbers.

I need to store these values dynamically in a double *note pointer for later use.

I tried the following code but it gives core dumped:

FILE *ifile = fopen("numbers.txt", "r");
double *note;
int i = 1; 

note = (double *) malloc( i * sizeof( double));
fscanf( ifile, "%lf", &note[0]); 

while ( !feof( ifile)) {
      i++;
      note = (double *) realloc( note, i * sizeof( double));
      fscanf( ifile, "%lf", &note[i]);
}       

for (n=0; n < i; n++) {
     printf( "%lf\n", note[i]);
}

[`while (!feof(...))` is always wrong](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). — melpomene, Jul 24 '16 at 17:41
@SergeBallesta the exercise specifies to not use a vector with a limited big size. — ovrwngtvity, Jul 24 '16 at 17:56
@SergeBallesta "Unrelated, but reallocing on each new value is an *anti-pattern*." In the realm of coding sins, calling `realloc()` for every loop iteration is about as minor an offense as possible. And I'm not even sure it should be counted as an *anti-pattern* at all - such code is much simpler and easier to write and maintain than code that `realloc()`'s larger chunks. It's just slower. Maybe. — Andrew Henle, Jul 24 '16 at 18:17

score 1 · Answer 1 · edited May 23 '17 at 12:22

Your code is accessing the array out of its bounds every time note[i] is used.

In the (wrong) while loop it's always one past the last element (for example in the first iteration i becomes 2, enough space for two doubles is allocated, but you access note[2] which is the third).

While printing, you use n as increasing loop index, but always print note[i] instead of note[n].

It would be also a good practice to check the return values of all the library function used, such as open, new, realloc and scanf.

A quick fix of those issues could be the following snippet. Please, note that I used the same reallocation strategy as yours (every time), but as @Serge Ballesta pointed out, this can be inefficient. Look at the alternative showed in @Jean-François Fabre answer, for example.

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

int main() {
    double value,
           *note = NULL,
           *newptr = NULL;

    int i,
        size = 0;

    char file_name[] = "numbers.txt";
    FILE *ifile = fopen(file_name, "r");
    if ( !ifile ) {
        fprintf(stderr, "Error while opening file %s.\n", file_name);
        exit(EXIT_FAILURE);
    }

    while ( fscanf(ifile, "%lf", &value) == 1 ) {
        size++;
        newptr = realloc(note, size * sizeof(double));
        if ( !newptr ) {
            fprintf(stderr, "Error while reallocating memory.\n");
            free(note);
            exit(EXIT_FAILURE);
        }
        note = newptr;
        note[size - 1] = value;
    }       

    for (i=0; i < size; i++) {
        printf( "%lf\n", note[i]);
    }

    free(note);     // <-- don't leak memory!
    fclose(ifile);
    return EXIT_SUCCESS;
}

Jean-François Fabre · Answer 2 · 2016-07-24T18:43:40.597

Your i passes from 0 to 2, and your index permanently is out of your boundary: you allocate 2 doubles, and write in index 2 which is the third. Also don't forget to close your file. AND the final print uses i where it should use n. It's not good to mix up usual letters (i for loop index, and n for limit), everybody gets confused in the end.

It's better to simplify/factorize your code as follows which avoids the bug:

As a bonus, I have added a mechanism which avoids reallocating each time, which is not very good performance-wise.

The following code has been tested and works

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

int main()
{
FILE *ifile = fopen("numbers.txt", "r");
double v,*note=NULL;
int i = 0,n; 
int alloc_step = 10;
int note_size = 0;

while ( !feof( ifile)) {
      fscanf( ifile, "%lf", &v);
      if ((i % alloc_step)==0)
      {
          note_size += alloc_step;
          note = (double *) realloc( note, note_size * sizeof( double));
      }
      note[i++] = v;

}       

for (n=0; n < i; n++) {
     printf( "%lf\n", note[n]);
}
fclose(ifile);
}

I changed the code but it is not working. It gives core dumped. I verified everything but it seems not working. — ovrwngtvity, Jul 24 '16 at 17:55

How to read values from an unspecified file size and store them dynamically in a vector in C?

2 Answers2