Please can you explain what this function does?

Question

Please can someone explain what this function does? And is it possible to code it more efficiently? I don't understand, especially, the if test.

What I can see: it is loading only numbers from a text file and those numbers are being stored to a pointer on the array

void LoadNumbers(int*pNumberArray,FILE*textFile) //---F7-
{
    int i = 0;
    while (fscanf(textFile,"%d", &pNumberArray[i]) != EOF)
          {
          if (fscanf(textFile,"%*[^-+0-9]", &pNumberArray[i]) == EOF) break;
          else if (pNumberArray[i] != 0 ){++i; pNumberArray[i] = '\0';
          }    
}

zmocho, what would you like to happen if input text is non-numeric and `fscanf(textFile,"%d", &pNumberArray[i])` returns 0? — chux - Reinstate Monica, Dec 09 '22 at 05:34
There is no reliable way for the calling function to tell how many records were loaded in the array. (The code messes around with keeping a zero at the end, but if a zero is read, chaos ensues — or any further data is ignored). There is no way for the function shown to know whether it is safe to store the data — it is not told how big the array is. The test for `!= EOF` is faulty; it should probably be `== 1` — though there are more elaborate possibilities. Each of those is a serious problem. You should find a source of better code than wherever it was this junk came from. — Jonathan Leffler, Dec 09 '22 at 05:42
Since `if (fscanf(textFile,"%*[^-+0-9]", &pNumberArray[i]) == EOF) break;` does not assign to anything (the `*` suppresses any assignment), the argument `&pNumberArray[i]` should not be passed to [`fscanf()`](https://pubs.opengroup.org/onlinepubs/9699919799/functions/fscanf.html). — Jonathan Leffler, Dec 09 '22 at 05:47

Allan Wind · Answer 1 · 2022-12-12T00:47:54.467

1

LoadNumbers() zero or more integers from textFile into pNumberArray. 0 values are skipped but a trailing zero is added to signify end of array. The input file consist of an integer optionally followed by something that is not +, - or a digit.

@ikegami pointed out that you should pass in the size of the array to avoid overflowing pNumberArray. Combine the two scanf() calls, add error handling, and just deal with the sentinel after the loop. (Not fixed) The function could alternatively return how many values are read (i) instead of using the 0 sentinel.

#include <stdio.h>

void LoadNumbers(size_t n, int pNumberArray[n], FILE *textFile) {
    if(!n) return;
    size_t i = 0;
    while(i + 1 < n) {
        int rv = fscanf(textFile,"%d%*[^0-9+-]", &pNumberArray[i]);
        if(rv == EOF) break;
        if(rv != 1) break; // error: no number found
        if(pNumberArray[i]) i++;
    }
    pNumberArray[i] = 0;
}

int main(void) {
    FILE *f = fopen("1.txt", "r");
    if(!f) {
        printf("fopen failed\n");
        return 1;
    }

    int a[100];
    LoadNumbers(sizeof a / sizeof *a, a, f);
    for(int i = 0; a[i]; i++) {
        printf("%d\n", a[i]);
    }
}

With the example 1.txt input file:

0 a
1 b
0 charlie charlie
2 d
3 echo
4 f
0 golf

Both the original and the revised function returns:

edited Dec 12 '22 at 00:47

answered Dec 09 '22 at 06:17

Allan Wind

23,068
5
28
38

1

`!i || !pNumberArray[i-1]` is always true. It's true when `i` is zero, and the only time `i` isn't than zero is if you've added a non-zero value to the array. Which makes sense. You always want to add a terminating zero, so the condition should always be true. – ikegami Dec 09 '22 at 17:47
@ikegami Yeah, I see what you are saying, while we may write a 0 in the loop, we don't advance `i` so the might as well just write it it unconditional 0 after the loop, which worst case overwrites it. Alternatively `if(!pNumberArray[i]) pNumberArray[i] = 0;` – Allan Wind Dec 09 '22 at 19:29
No, `if(!pNumberArray[i]) pNumberArray[i] = 0;` is wrong. You might never have assigned to `pNumberArray[i]`, so that results in UB. – ikegami Dec 09 '22 at 20:14
@ikegami op did not tell us the specific c version, if c11, apparently https://stackoverflow.com/questions/11962457/why-is-using-an-uninitialized-variable-undefined-behavior it's only an issue reading an uninitialized value if it's a declared a register (or trap representation which seems esoteric). If you declare `register a[LEN]` gcc won't compile the program in particular it says the call to the function itself requires taking the address of `a`. What am I missing? – Allan Wind Dec 10 '22 at 02:01
1

If it *could* have been declared as a register. Not if it did. But seeing as its address was taken, it couldn't have been declared as a register. I guess you're right. It's still UB, cause it might happen to be a trap representation, but x86/x86_64 doesn't have that. So I guess `if(!pNumberArray[i]) pNumberArray[i] = 0;` is fine on that architecture. – ikegami Dec 10 '22 at 02:17
,%d%*[^-+0-9]", please can u explain me how exactly this format string in fscanf works ? bcs ^ means (Only characters ^not listed between the brackets are accepted) so why does it actually read these characters anyway ? – grepawk Dec 11 '22 at 20:55
%d means read an intt; %*[^0-9+-] means read and throw the result away what doesn't match the 0-9+-. – Allan Wind Dec 12 '22 at 00:45
Sorry for bothering u :D but why there is * and ^ (and why it doesnt do exactly the opposite cuz ^ means not listed between the brackets are accepted) – grepawk Dec 12 '22 at 05:27
The `*` means ignore whatever it reads. This is why there is no matching variable. In summary the total format string means read a int then ignore anything that is not a -, + or a digit (this is loosely an int). So "1x" will read 1 and ignore the x", "12 +x" will read 12, ignore the space, then in time read the + and probably generate an error as that is not a number but ignore the x. – Allan Wind Dec 12 '22 at 05:32
"12 12" would read an int, ignore the space, then read the int in the next loop iteration. – Allan Wind Dec 12 '22 at 05:38

Please can you explain what this function does?

1 Answers1