Clang-Tidy: 'fscanf' used to convert a string to an integer value, but function will not report conversion errors; consider using 'strtol' instead

Question

Somewhat unexperienced with C here!

I'm using CLion to write a program and keep getting this warning message whenever I use fscanf to store a value from an input file into a variable:

Clang-Tidy: 'fscanf' used to convert a string to an integer value, but function will not report conversion errors; consider using 'strtol' instead

I don't understand this error as I thought fscanf was the function I should be using to read input files? Can someone explain (at a noob level) what's wrong with the way I'm using it?

Here's a sample of my code:

FILE *initial_configuration_box_1_pointer;
initial_configuration_box_1_pointer = fopen("initial_configuration_box_1.xyz", "r");
fscanf(initial_configuration_box_1_pointer, "%d\n", &N1); // Warning here.
fscanf(initial_configuration_box_1_pointer, "pid\tx\ty\tz\n"); // No warning here.
for (i = 1; i <= n1; i++)
{
    fscanf(initial_configuration_box_1_pointer, "p\t%lf\t%lf\t%lf\n", &rx1[i - 1], &ry1[i - 1], &rz1[i - 1]); // Warning here.
}
fclose(initial_configuration_box_1_pointer);

I'm aware similar questions have been asked, but I couldn't understand any of the (few) answers they got...

Note: conversion errors include: no conversion, (input not numeric text), range (Input outside `int` range), trailing junk after text. — chux - Reinstate Monica, Jun 01 '21 at 19:24
`fscanf` is never the right tool. But...if you're going to use it, you *must* check that it actually did something. eg, `if( fscanf(...,"%d", &N1) == 1)` (Don't put whitespace after the conversion specifier). See http://sekrit.de/webdocs/c/beginners-guide-away-from-scanf.html — William Pursell, Jun 01 '21 at 20:21
The short answer is: `scanf` (likewise `fscanf`) *sucks*. These functions have one use and one use only: reading simple interactive input in (a) test programs written by (b) those learning C for the first time, and (c) during the first, say, two weeks of their learning curve, when everything is new and there's only so much you can learn per day. But as soon as you're ready, learn about the [better alternatives](https://stackoverflow.com/questions/58403537/) and abandon those two benighted functions like the square training wheels they are. — Steve Summit, Jun 01 '21 at 21:02
@WilliamPursell, I tried something along the lines of: `if (fscanf(initial_configuration_box_1_pointer, "%d", &N1) == 1) fscanf(initial_configuration_box_1_pointer, "%d", &N1);` and I'm still getting the same error. Is that what you meant I should do? — Felipe Evaristo, Jun 03 '21 at 19:57
@SteveSummit Thanks for the thread you pointed to! I couldn't figure out how to apply those other tools to my specific case, though. I have input coming from a file (not the terminal) that contains multiple entries per line separated from each other by whitespaces. Each line looks like this: '10000 -54.66883 -14.94840 -12.90989 -40.67234 0.56056 0.01092 706.44203 10625.62156 11332.06359 396 116 512` I must import each value into an entry of an array. My understanding is the tools you pointed to require a single entry per line. How can I avoid using `fscanf` in my specific case? — Felipe Evaristo, Jun 03 '21 at 20:02
@FelipeEvaristo No. You do not want to call `fscanf` twice. Just call it once, but you must check that it successfully read something. For example, if the next character in the input stream is `q`, then `scanf("%d", &N1);` will return 0 and not write anything to N1. If you don't check the value returned by `scanf`, you will miss input errors like that. — William Pursell, Jun 03 '21 at 21:30
@FelipeEvaristo In [that other thread](https://stackoverflow.com/questions/58403537), read [my long answer](https://stackoverflow.com/questions/58403537/what-can-i-use-for-input-conversion-instead-of-scanf/58405726#58405726) starting where it says "The rest of the main narrative concerns input you might be trying to parse that's more complicated than just a single number or character." — Steve Summit, Jun 04 '21 at 10:58
@WilliamPursell Sorry for the delay in getting back to you all. COVID times have been crazy and I could only get back to this now. I tried sticking with `fscanf` making sure to check for conversion errors, but Clang-Tidy keeps complaining about it. I figured I should probably move on to solutions not involving `fscanf` based on the amount of criticism I've found for it... — Felipe Evaristo, Jul 31 '21 at 15:26
@SteveSummit Sorry for the delay in getting back to you all. COVID times have been crazy and I could only get back to this now. Thanks for the thread you pointed me to! I checked your long post thoroughly and tried to implements your solutions. However, now, I'm facing the problem that `fgets` isn't working for me. It isn't reading my input file line-by-line as it should because (I think) it isn't identifying the end of each line of that file correctly. I'm starting a new post to deal with that particular problem. I'll link to it here in a minute if you're interested and can check it out. — Felipe Evaristo, Jul 31 '21 at 15:32

William Pursell · Answer 1 · 2021-06-03T13:16:11.720

There are a lot of good reasons for a beginner to avoid scanf completely. (Read http://sekrit.de/webdocs/c/beginners-guide-away-from-scanf.html). If you're going to use it interactively, never end a format string with whitespace, since doing so will just confuse the user. And always check the value returned by scanf to see if it actually matched any input. A common error is for the input to fail to match the expected data, and the scanf loop becomes an infinite loop repeatedly checking the same invalid input. Perhaps the warning is suggesting the second point above: since scanf does not validate the input for you, you have to do it explicitly by checking how many conversion specifiers scanf was able to match. Try something like:

#include <stdio.h>
#include <stdlib.h>

int
main(int argc, char ** argv)
{
        int n1;
        const char *path = argc > 1 ? argv[1] : "initial_configuration_box_1.xyz";
        FILE *ifp;
        if( (ifp = fopen(path, "r")) == NULL ){
                perror(path);
                return EXIT_FAILURE;
        } 
        if( 1 != fscanf(ifp, "%d", &n1) || n1 <= 0 ){
                fprintf(stderr, "Invalid input\n");
                return EXIT_FAILURE;
        }
        fscanf(ifp, " pid x y z");
        double rx1[n1];
        double ry1[n1];
        double rz1[n1];

        for( int i = 0; i < n1; i++ ){
                if( 3 != fscanf(ifp, " p %lf %lf %lf", rx1 + i, ry1 + i, rz1 + i) ){
                        fprintf(stderr, "Invalid input near line %d\n", i);
                        return EXIT_FAILURE;
                }               
        }
        if( fclose(ifp) ){
                perror(path);
                return EXIT_FAILURE;
        }
        return EXIT_SUCCESS;
}

Note that whitespace in a scanf format string does not match exactly, so using \n or \t or is all the same. Usually, people just use a single space to make it easier to read. Also, whitespace between certain conversion specifiers (notable %d and %lf) is irrelevant, and only included for readability.

Clang-Tidy: 'fscanf' used to convert a string to an integer value, but function will not report conversion errors; consider using 'strtol' instead

1 Answers1