1

I'm having an aggravating issue that I can't understand,

void file_count(FILE* stream,int* const num)
{
    int temp;
    while((fscanf(stream,"%d",&temp))!=EOF)
    {
        (*num)++;
    }
}

With this piece of program, I read from a file taking all the numbers inside it, and very time I take a number a counter increases so I can count how many elements are in the file.

In this file there are 6 numbers, but if I run this code the counter skyrockets to 32777... If I compile it with gcc, there's no problem and the counter is 6 as expected. Is this a bug of clang? Is it a feature that I'm not aware of?

The file contains:

22 30 30 21 25 29

The whole code:

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

typedef char* string;
typedef struct student
{
    int flag;
    char name[25];
    char surname[25];
    char dorm[25];
    int* grades;
}
Student;

void check_input(const int argc,const string* const argv);
void check_file(FILE* stream);
void file_count(FILE* stream,int* const num);
void update_student(const string* const infos,Student* const student,const int grades,FILE* stream);
void print_student(FILE* stream,Student const student,const int grades);

int main(int argc, string argv[])
{
    check_input(argc,argv);
    FILE* one,* two;
    string info[]={"David","Malan","Mather"};
    Student student;
    int grades;
    one=fopen(argv[1],"r");
    check_file(one);
    file_count(one,&grades);
    update_student(info,&student,grades,one);
    fclose(one);
    two=fopen(argv[2],"w");
    check_file(two);
    print_student(two,student,grades);
    fclose(two);
    free(student.grades);
    system("cat out");
    return 0;
}

void check_input(const int argc,const string* const argv)
{
    if(argc!=3)
    {
        printf("\x1B[31mError: %s takes two arguments!\x1B[0m\n",argv[0]);
        exit(EXIT_FAILURE);
    }
}

void check_file(FILE* stream)
{
    if(stream==NULL)
    {
        printf("\x1B[31mError:invalid file.\x1B[0m\n");
        exit(EXIT_FAILURE);
    }
}

void file_count(FILE* stream,int* const num)
{
    int temp;
    printf("reading file...\n");
    while((fscanf(stream,"%i",&temp))!=EOF)
    {
        (*num)++;
    }
    printf("\x1B[33mthe value read were %i\x1B[0m\n",*num);
}

void update_student(const string* const infos,Student* const student,const int grades,FILE* stream)
{
    rewind(stream);
    student->grades=malloc(grades*sizeof(int));
    strcpy(student->name,infos[0]);
    strcpy(student->surname,infos[1]);
    strcpy(student->dorm,infos[2]);
    student->flag=0;
    for(int i=0;i<grades;i++)
    {
        fscanf(stream,"%i",&student->grades[i]);
    }
}

void print_student(FILE* stream,Student const student,const int grades)
{
    printf("Writing to file..\n");
    fprintf(stream,"%i %s %s %s ",student.flag,student.name,student.surname,student.dorm);
    for(int i=0;i<grades;i++)
    {
        fprintf(stream,"%i ",student.grades[i]);
    }
    printf("\x1B[32mFile successfully written..\x1B[0m\n");
}
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • Could be any number of things wrong in the code that you haven't shown. See [minimal, complete, and verifiable example](https://stackoverflow.com/help/mcve). – user3386109 Dec 15 '17 at 19:24
  • Show more code and, preferably, the file you use as input. – AnT stands with Russia Dec 15 '17 at 19:26
  • Probably hitting something that isn't an integer (most likely end of line, in fact), so `fscanf` doesn't consume it, so you get get stuck there. – Fred Larson Dec 15 '17 at 19:26
  • 1
    Um, `*scanf()` family of functions return _number of items scanned_. The `EOF` constant shouldn’t be any where near it. – Dúthomhas Dec 15 '17 at 19:32
  • @user3121023 doesn't work :/ – Giannandrea Vicidomini Dec 15 '17 at 19:32
  • Given that you've got a problem, capture the actual return value of `fscanf()` and (in the loop), print the return value and the value that was read: `void file_count(FILE* stream,int* const num) { int temp; int rc; printf("reading file...\n"); while((rc = fscanf(stream, "%i", &temp)) != EOF) { (*num)++; printf("%d: rc=%d [%d]\n", *num, rc, temp); } printf("\x1B[33mthe number of values read were %i\x1B[0m\n", *num); }` — or similar. – Jonathan Leffler Dec 15 '17 at 19:57
  • By the way you can select the answer which helped you by clicking the green tick. – user2736738 Dec 26 '17 at 20:12

2 Answers2

4

Your code is dangerous, because an incorrect file sends it into an infinite loop.

Once fscanf with %d finds an input that cannot be interpreted as an int, the function returns zero without making any progress on consuming the input. Therefore, the loop never reaches EOF.

You can fix this issue by looping only as long as the input is consumed:

while(fscanf(stream,"%d",&temp) == 1) {
    ...
}

Now you need a way to communicate to the caller if the count is correct or not. You can do that by returning one if EOF is reached, and zero otherwise:

int file_count(FILE* stream,int* const num) {
    int temp, last;
    while((last = fscanf(stream,"%d",&temp)) == 1) {
        (*num)++;
    }
    return last == EOF;
}

I tried with fscaf==1 it still reaches 32777

This happens because you are not initializing grades in the caller. You should define it as int grades = 0, or add *num = 0 before entering the loop in file_count.

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • 1
    @GiannandreaVicidomini That's because you are not initializing `grades` in the caller. You should put `int grades = 0`. – Sergey Kalinichenko Dec 15 '17 at 19:38
  • 1
    @FredLarson You are right, I added space after `%d` to make sure the ending space is consumed. – Sergey Kalinichenko Dec 15 '17 at 19:39
  • @dasblinkenlight i'm an idiot, now i see, the garbage value... but now another question is due... why does gcc works correctly? and also why the garbage value never changes? – Giannandrea Vicidomini Dec 15 '17 at 19:40
  • @GiannandreaVicidomini: Probably because undefined behavior includes the possibility of the code seeming to work correctly. – Fred Larson Dec 15 '17 at 19:41
  • @GiannandreaVicidomini gcc "works" fine because the garbage value that it uses happens to be zero. This visibility of "correct behavior" creates a lot of problems with undefined behavior. – Sergey Kalinichenko Dec 15 '17 at 19:42
  • @dasblinkenlight There's no need for a space after the `%d`. Every conversion (except `%c`) consumes leading white space before starting the conversion. So the newline from the previous `%d` is consumed as leading white space by the next `%d`. – user3386109 Dec 15 '17 at 19:47
  • 1
    @user3386109 Thank you very much for the comment! I did not realize that this is what was happening. I posted [a question](https://stackoverflow.com/q/47839125/335858) to demonstrate this point. – Sergey Kalinichenko Dec 15 '17 at 20:03
  • You're welcome. I found the question and answer very informative. I had forgotten about `%[]` and `%n`, but §7.21.6.2/8 confirms that those (plus `%c`) are the only three. Good question. – user3386109 Dec 15 '17 at 20:20
0

The current ISO C standard defines the return value of fscanf() as

The fscanf function returns the value of the macro EOF if an input failure occurs before the first conversion (if any) has completed. Otherwise, the function returns the number of input items assigned, which can be fewer than provided for, or even zero, in the event of an early matching failure.

Verbatim quote from the ISO-C Standard (no public link available, the standard is not public AFAIK)

If there is data to read, but that data cannot be successfully matched, this function will return 0. It will only return EOF (which is typically -1) if there is no data to read because the stream you are reading from is in EOF state or had another read error.

So if you had a file like that:

23 a 57

The loop will never terminate. On first scan it will read 23 and return 1 (one value matched). On next call it will return 0, as a cannot be matched to an integer. Yet, it also won't move the file pointer beyond a. So on next call, it tries to match a again and it will fail again. This continues endlessly.

Mecki
  • 125,244
  • 33
  • 244
  • 253
  • @FredLarson You are correct, the other site didn't mention that case; I have a copy of the ISO C standard (C11), so I replaced the quote with one from that standard which must be correct by definition (it's the standard after all, it defines what is right and what is wrong). – Mecki Dec 15 '17 at 19:57