Why placement of declaration of variable is important?

Question

The main task for my code is to take out data from outside files and then ascribe a number at the beginning. The problem is that the code below works just fine.

#include <stdio.h>

int main(void) {
    char name[20], surname[30], group[2];
    puts("Now i download data from file");
    int n = 1, number;
    while (!feof(stdin)) {
        scanf ("%s %s %d %s", name, surname, &number, group);
        printf("%d. %s | %s | %d | %s", n, name, surname, number, group);
    }
    return 0;
}

But when I change the position of one line it sets the value of n to 0 and doesn't rise.

#include <stdio.h>

int main(void) {
    int n = 1, number;
    char name[20], surname[30], group[2];
    puts("Now I download data from file");
    while (!feof(stdin)) {
        scanf ("%s %s %d %s", name, surname, &number, group);
        printf("%d. %s | %s | %d | %s", n, name, surname, number, group);
    }
    return 0;
}

Are you (perhaps) trying to read more than one character into the `group` variable? (You will need an 'extra' character to hold the 'nul' terminator.) This *may* cause some sort of 'overrun' into the memory assigned to the 'n' variable - but I can't be sure. — Adrian Mole, Mar 11 '20 at 16:14
@AdrianMole that was the issue, thank you so much, but I don't get why placing that one line lower helps the problem — cover, Mar 11 '20 at 16:34
How (and where) the `Xcode` compiler assigns memory for your variables is not something I can claim to understand. However, overrunning the buffer assigned to your `group` variable *will* cause **undefined behaviour** and, at that point, literally anything can happen - including code that *seems* to work. — Adrian Mole, Mar 11 '20 at 16:38
@AdrianMole can I do something to declare name, surname and group without setting maximum size? For example I don’t know the name of group and sometimes it can has 4 characters and sometimes 10. — cover, Mar 11 '20 at 16:51
The simplest (and perhaps most common) approach is to declare your string arrays such that they are large enough to cover all possible input values. On top of this, you can add a digit before the `s` in the `scanf` format to limit the maximum number of characters to read, like `scanf("%20s %30s %d %10s"...` — Adrian Mole, Mar 11 '20 at 17:00
Note that [`while (!feof(file))` is always wrong!](https://stackoverflow.com/q/5431941/15168). You must check that the `scanf()` worked before using the results it was supposed to collect: `if (scanf(…) == 4) { …OK… } else { …deal with problem… }`. — Jonathan Leffler, Mar 11 '20 at 17:09
You could see if your implementation supports the `%ms` extension to `scanf`, where the function will automatically allocate you an array of sufficient size. — Nate Eldredge, Mar 11 '20 at 17:23
Note it's pretty common in C that when you have something like a buffer overflow, then apparently unrelated changes to your program often result in unpredictable changes in its behavior, including the possibility that it may appear to work correctly. When you observe this, the right question to ask yourself is not "what combination of unrelated tweaks makes it work again" but "what is the underlying problem". — Nate Eldredge, Mar 11 '20 at 17:32

score 2 · Accepted Answer · edited Mar 13 '20 at 01:41

Your code has potential undefined behavior: if the data file contains fields that exceed the maximum length storable in the destination arrays, fscanf() will cause a buffer overrun and overwrite something else in memory, possibly another local variable.... Changing the order of the local variable definitions may change the side effects of this bug. Undefined behavior can lead to anything, from no visible effect to the program crash or even a market crash.

Also note that while (!feof(stdin)) is always wrong.

Here is how you can modify your code to avoid this undefined behavior and verify that the data file is correctly converted:

#include <stdio.h>

int main(void) {
    char buf[128];
    char name[20], surname[30], group[2];
    int n = 1, number;
    char dummy;

    puts("Now I download data from file");

    while (fgets(buf, sizeof buf, stdin)) {
        if (sscanf("%19s %29s %d %1s %c", name, surname, &number, group, &dummy) == 4) {
            printf("%d. %s | %s | %d | %s\n", n++, name, surname, number, group);
        } else {
            printf("Invalid format: %s", buf);
        }
    }
    printf("%d records\n", n);
    return 0;
}

Any records with a group larger than a single character will cause a format error. If the group may have 2 characters, you should define the array as char group[3] and convert it with %2s. The same remark applies to all string fields.

Why placement of declaration of variable is important?

1 Answers1