0

I have a code in which I am getting warning of unchecked return value-

bool check() {
    FILE* fptr = fopen(fi.txt, "r");

    if(fptr != NULL) {
        while(!feof(fptr)) {
            fscanf(fptr, "%s", var1);

            if(strcmp(fptr, var2) == 0) {
                fclose(fptr);
                return true;
            }
        }

        fclose(fptr);
    }

    return false;
}

I have found a way to rewrite this code as I am getting warning like this

   Calling fscanf(fptr, "%s", var1) without checking return value. This library function may fail and return an error code.

Here is another way, is this thr right way or not-

if(fptr != NULL) {
    while(fscanf(fptr, "%s", var1) == 0) {
        if(strcmp(var1, var2) == 0) {
            fclose(fptr);
            return true;
        }
        fclose(fptr);
    }
    return false;
}
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
Abhay Singh
  • 407
  • 1
  • 4
  • 14
  • 1
    No, read the documentation for `fscanf`. It will return the number of successful "scans". You should expect it to return `1` if your scan is successful, not `0`. – Ted Lyngmo Apr 01 '21 at 07:14
  • 2
    Also [`while (!feof(file))` is always wrong](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong?r=SearchResults&s=1|512.3432) – pmg Apr 01 '21 at 07:20
  • Another note. I came to this question because of another question you had that was closed because you didn't leave enough information. This question has the same problem. You didn't tag it with any programming language and people rarely subscribe to the `static-analysis` tag alone. Also, make [mre]s. It will help people to help you. – Ted Lyngmo Apr 01 '21 at 07:21

1 Answers1

0

Do not use while(feof). Instead just check if reading the actual data was successful.

Don't:

    while(!feof(fptr)) {
        fscanf(fptr, "%s", var1);

Just do:

    while(fscanf(fptr, "%s", var1) == 1) {

I do not like nested ifs error handling. I would most probably prefer a short error handling statements with single return point as I was influenced by kernel style:

bool check() {
    FILE* fptr = fopen(fi.txt, "r");
    if (fptr == NULL) goto fopen_failed;   // short error handling with goto-s

    bool ret = false;
    while(fscanf(fptr, "%s", var1) == 1) {
        if(strcmp(fptr, var2) == 0) {
           ret = true;
           break;
        }
    }
    fclose(fptr);

fopen_failed:
    return ret;    // single return point
}
KamilCuk
  • 120,984
  • 8
  • 59
  • 111