EOF in while jumps out of the scope

Question

I have the below code which is supposed to read a file line by line and do a for on each line but it jumps out of the while after first time of running:

while (fscanf(f, "%s%*c%d\n", serial, &n) != EOF) {

    for (j = 0; j < i + 1; j++) {
        if (strcmp(serial, listProducts[j].serialID) == 0) {
            listProducts[j].n_items += n;
        } else {
            strcpy(listProducts[i].serialID, serial);
            listProducts[i].n_items = n;
        }
    }
    i++;
}

the input is a file with the below information: a.txt

OP001Y100SSID 11 \n
AP003XMCN90ID 10 \n
BP007QRV200ID 19 \n
OP002HJDE28ID 22 \n
CP009QWDGREID 21

Why do you think that the number of successful conversions returned from `fscanf` might ever be equal to `EOF` (which is usually negative)? — Toby Speight, Jul 19 '23 at 16:23
Your `fscanf` format is overly complicated, `"%s %d"` will be just fine (depending on format of input). And you should check that `fscanf` [returns](https://en.cppreference.com/w/cpp/io/c/fscanf#Return_value) `2` instead. — Some programmer dude, Jul 19 '23 at 16:25
Or better yet, use `fgets` to read a full line, and then use e.g. `sscanf` to parse the line (using the format I shown). By using `fgets` it's also much easier to debug your program because you can see the actual input. — Some programmer dude, Jul 19 '23 at 16:25
By the way, what *is* your input? Please show the contents that causes your program to fail. Or enough for us to better understand the possible problem. — Some programmer dude, Jul 19 '23 at 16:27
@TobySpeight: `EOF` is a specified return value from `fscanf`. — Eric Postpischil, Jul 19 '23 at 16:49
@MaryamGhafarinia `serial[]` is too small. Post its definition. A [mcve] is even better. — chux - Reinstate Monica, Jul 19 '23 at 16:53
It is usually better to check the actual number of conversions, e.g. `while (fscanf(f, "%s%d", serial, &n) == 2)` and if you need to disambiguate invalid data from EOF save the return value in a variable, e.g. `while ((retval = fscanf(f, "%s%d", serial, &n)) == 2)`. Note that I also removed the trailing newline from the format string. — Weather Vane, Jul 19 '23 at 16:54
Please [edit] your question and show a [mre]. Please check if the `\n` is really part of the input and fix or confirm this. How exactly do you detect that "*it jumps out of the while after first time of running*"? (Step through your code in a debugger or add some `printf`s to see what it's doing.) — Bodo, Jul 19 '23 at 16:56
You increment `i` on every iteration of the `while` loop, but you don't increase the list size on every iteration. And the `else` clause of the `if` statement needs to be after the `for` loop. The `if` itself needs a `break` statement. — user3386109, Jul 19 '23 at 16:56
Note [What is the effect of trailing white space in a `scanf()` format string?](https://stackoverflow.com/q/19499060/15168) — Jonathan Leffler, Jul 19 '23 at 17:52

chqrlie · Answer 1 · 2023-07-19T18:39:03.917

The purpose of this parsing loop is to update the list of products from the contents of the file. The while loop has problems:

the fscanf() format uses %s which is risky as unexpectedly long input may cause a buffer overflow, attempting to write beyond the end of the serial destination array.
comparing the fscanf() return value to EOF is not reliable: you should instead check the number of successful conversions, which should be 2.
the \n at the end of the format string matches any white space, it is not a reliable way to test for the end of line.
the for loop attempts to locate the product in the listProducts array and add an entry if it was not found. The loop logic is incorrect for this purpose: you always increment i.

It is recommended to read the file one full line at a time with fgets(), parse the line with sscanf() and report any parsing issues.

Here is a modified version:

int load_products(struct product *listProducts, int count, FILE *f) {
    char buf[128];
    int i = 0;

    while (fgets(buf, sizeof buf, f)) {
        char serial[30];
        int n;
        int j;

        if (sscanf(buf, "%29s%d", serial, &n) != 2) {
            fprintf(stderr, "invalid format: %s\n", buf);
            continue;
        }

        for (j = 0; j < i; j++) {
            if (strcmp(serial, listProducts[j].serialID) == 0)
                break;
        }
        if (j < i) {
            /* product was found: update the count */
            listProducts[j].n_items += n;
        } else {
            /* product was not found: add an entry at the end */
            if (i >= count) {
                fprintf(stderr, "too many products, maximum: %d\n", count);
                break;
            }
            strcpy(listProducts[i].serialID, serial);
            listProducts[i].n_items = n;
            i++;
        }
    }
    return i;
}

EOF in while jumps out of the scope

1 Answers1