2
#include <stdio.h>

int main()
{
    FILE* f=fopen("book2.txt","r");
    char a[200];
    while(!feof(f))
    {
        fscanf(f,"%s",a);
        printf("%s ",a);
        printf("%d\n",ftell(f));
    }
    fclose(f);
    return 0;
}   

I have the code above. book2.txt contains "abcdef abcdef" with the cursor move to a newline(ie:abcdef abcdef\n). I get the results below.

abcdef 6
abcdef 13
abcdef 19

I expect to get

abcdef 6
abcdef 13
15

What am I doing wrong?

joy
  • 721
  • 2
  • 11
  • 18

2 Answers2

4

Test the I/O operations themselves; don't use feof() unless you're writing Pascal (and C is not Pascal!)

#include <stdio.h>

int main(void)
{
    FILE *f = fopen("book2.txt", "r");
    char a[200];
    if (f != 0)
    {
        while (fscanf(f, "%199s", a) == 1)
        {
            printf("%s ", a);
            printf("%ld\n", ftell(f));
        }
        putchar('\n');
        fclose(f);
    }
    return 0;
}

Note that the revised code tests f before using it, and protects against buffer overflow by specifying how long the string is in the conversion specification. Be aware that %s reads up to a white space character; it does not read lines unless there is no white space on each line.

You use feof() to distinguish between a conversion failure, an I/O error and EOF after an operation such as fscanf() reports trouble. For example:

#include <stdio.h>

int main(void)
{
    FILE *f = fopen("book2.txt", "r");
    char a[200];
    if (f != 0)
    {
        while (fscanf(f, "%199s", a) == 1)
        {
            printf("%s ",a);
            printf("%ld\n", ftell(f));
        }
        putchar('\n');
        if (feof(f))
            printf("EOF\n");
        else if (ferror(f))
            printf("I/O error\n");
        else
            printf("Conversion failed\n");
        fclose(f);
    }
    return 0;
}

With %s, you won't get to conversion failed, and I/O error is pretty improbable too. If the conversion specifier was %d, though, a punctuation character in the data could land you with 'Conversion failed'.

In over 25 years worth of C coding, I've got 2 places in my code that use feof() out of thousands of files (I did a check a few months ago), and in both cases, the code is distinguishing between EOF and error (roughly as shown).

Community
  • 1
  • 1
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
2

feof will not tell you that you've hit the end of the file until after you've hit the end of the file. Check the result (return value) from fscanf. It will tell you when it cannot read any more. If fscanf hits the end of file, it will return EOF (and feof will return true after this has happened).

Now just for fun, take a look at this way of doing it. Don't actually ever do it this way.

while(1) {
    switch(fscanf(f,"%s",a)){
        case 1: 
            printf("%s ",a);
            printf("%d\n",ftell(f));
            continue;
        case EOF:
            break;
    }
    break;
}

This way is wrong because it is clever. With a few more cases, the behavior of the loop as a whole would become very difficult to discern (eg. this program). With fscanf you really only need to know if you got all the data you need to continue, or not. Let the next input reading function deal with EOF when it finds it. This call to fscanf needs to fill x number of values so,

if (fscanf(f,"%d %d %d", &i, &j, &k) != 3)
    /* cannot continue. need i j k values for next computation */

then do your error-response, or exit the loop, or whatever your program needs to do next. But if fscanf returns anything other than 3, it did not read 3 values.

Community
  • 1
  • 1
luser droog
  • 18,988
  • 3
  • 53
  • 105
  • Please explain to us what's wrong with *this way*, so that we know what to do differently, and you have my vote. – autistic Apr 07 '13 at 05:54
  • Ok. I tried. I think I matured a little while writing that. :) – luser droog Apr 07 '13 at 06:05
  • The WTF there is a loop that only loops once, which isn't relevant to this question. Consider `int foo; for (foo = fscanf(stdin, "%s", bar); foo == 1; foo = fscanf(stdin, "%s", bar)) { printf("%s\n", bar); }`... After this loop, one can check the value returned to see if it was a match failure (which can't happen for this format string) or a read failure that lead to fscanf returning something other than 1. – autistic Apr 07 '13 at 06:28
  • I don't understand what you're saying here. The WTF *where?* – luser droog Apr 07 '13 at 06:32
  • I guess what I'm trying to say is: Aim to teach people how to do things the correct way, rather than teaching people what the incorrect way is. "The WTF" is a reference to a website named "the daily WTF". – autistic Apr 07 '13 at 06:53
  • Agreed. But it seemed Jonathan Leffler already had that covered. All that was left was *something interesting to think about.* – luser droog Apr 07 '13 at 06:57
  • @modifiablelvalue This is actually a step forward for me. Not too long ago, I would have called the same code *good.* – luser droog Apr 09 '13 at 04:38