fscanf-- trying to scan txt file of ints, fscanf only reads 1s

Question

I'm trying to use fscanf to read a file containing 25 ints and store them in memory. However, it appears that for the first 12 values, instead of scanning the actual int in the file fscanf is always showing up as 1. The 13th value shows up as -1, and then the while loop in the code below terminates. Any idea why this might be? Thanks for your help.

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

#define ROWS 5
#define COLS 5

void load_file(FILE* file, int** p);

int main()
{


    FILE* f1;
    f1 = fopen("twenty-five-ints.txt", "r");
    int p=0;
    int* matrix = &p;
    load_file(f1, &matrix);

}

void load_file(FILE* file, int** p) {

    *p = malloc(25*sizeof(int));

    int number = 0;
    int i = 0;

    while (fscanf(file, "%d", &number) != EOF) {
        *(*p + i) = fscanf(file, "%d", &number);
        printf("%d ", *(*p + i));
        i++;
    }
    printf("\n");
}

The printf statement inside the while loop prints out 12 ones separated by spaces, followed by a -1.

Answer 1

There are two things to mention.

1. Remove one fscanf() call. Othersise, you'll end up losing every alternative value scanned.
2. fscanf() does not return the scanned value. In case a macth is found, it stores the scanned value in the supplied argument (&number). Use the argument to get the scanned value. You can make use of the return value to check for the suucess os the call to fscanf().

Quoting the man page, (emphasis mine)

The scanf() family of functions scans input according to format as described below. This format may contain conversion specifications; the results from such conversions, if any, are stored in the locations pointed to by the pointer arguments that follow format. [...]

Answer 2

You should not fscanf() twice and you should compare fscanf() to the number of expected fields to be scanned.

while ((i < 25) && (fscanf(file, "%d", (*p + i)) == 1))
    printf("%d ", *(*p + i++));

Also, fscanf() does not return the scanned value, what would you expect it to return in this case?

fscanf(file, "%s%d%", &string, &integer);

Aditionally Consider:

1. Using index notation to dereference the pointer.
2. Using an aditional pointer to avoid confusion.
3. Checking the return value from malloc()

---

void 
load_file(FILE *file, int **data)
{
    int *pointer;
    size_t i;

    *data = NULL; // So you can test this after the call to the function

    pointer = malloc(25 * sizeof(**data));
    /*                ^ if this is a constant, this doesn't make a lot of sense */
    /*                  because you can use an array instead.                   */
    if (pointer == NULL)
        return;

    for (i = 0 ; ((i < 25) && (fscanf(file, "%d", &pointer[i]) == 1) ; ++i)
        printf("%d ", pointer[i]);
    printf("\n");

    *data = pointer;
}

In my opinion, this function is poorly designed. You can't verify if the read values where in fact 25, nor can you specify that anywhere. If you want a function to read a given number of integers with a maximum try this

size_t
load_file(FILE *file, int **data, size_t maximum)
{
    int *pointer;
    size_t i;

    *data = NULL; // So you can test this after the call to the function

    pointer = malloc(maximum * sizeof(**data));
    if (pointer == NULL)
        return;

    for (i = 0 ; ((i < maximum) && (fscanf(file, "%d", &pointer[i]) == 1) ; ++i)
        ;
    *data = pointer;
    return i;
}

With this function, you can do this

int *data;
// We assume that FILE * is a valid stream
size_t count = load_file(file, &data, 25);
if (data != NULL)
{
    for (size_t i = 0 ; i < count ; ++i)
        fprintf(stdout, "%d ", data[i]);
    fputc('\n', stdout);
    free(data);
}