Return value for opening non-existing file failing in C-CodePudding

I am currently working on a program and it's supposed to return 2 if the input file can't be opened or doesn't exist, but I'm getting a segmentation fault instead. The existing input file already exists in my folder and it's supposed to be opened when its name is written as a parameter as shown in the code:

        int max_plateau = 0;
        for (int i = 1; i < argc - 1 ; i  ) {
            FILE *input = fopen(argv[i], "r");
            if (ferror(input)) {
                return 2;
            } else {
                read_file(input, &bridges, &count, max_plateau);
                if (max_plateau != 0) {
                    bridges[max_plateau - 1]  = 1;
                    bridges[max_plateau]  = 1;
                }
                max_plateau = count;
                fclose(input); 
            }
        }

The same goes for the output file

        if (argv[argc - 1] == NULL)
            return 2;
        else {
            int odd = 0;
            find_result(&ungerade, anzahl, &bruecken);
            FILE *output = fopen(argv[argc - 1], "w");
            if (ferror(output)) {
                if (bridges != NULL) {
                    free(bridges);
                }
                return 2;
            } else {
                print(output, odd, &bridges, count);
                fclose(output);
            }
        }

CodePudding user response：

fopen() returns a null pointer if it cannot open the file.

Calling ferror() with a null pointer causes undefined behavior as you observe.

ferror() cannot be used for this, this function accesses the error indicator in the FILE structure that may be set by an operation on the stream that fails. For example getc() returns EOF if the stream is at end of file or if there was some kind of error on the stream. Calling ferror(file) or feof(file) can help determine what happened, but these functions are almost never used. Calling feof() for anything else, such as before a file operation to test if the end of file has been reached is a common mistake.

Here is a modified version:

        int max_plateau = 0;
        for (int i = 1; i < argc - 1 ; i  ) {
            FILE *input = fopen(argv[i], "r");
            if (input == NULL) {
                return 2;
            } else {
                read_file(input, &bridges, &count, max_plateau);
                if (max_plateau != 0) {
                    bridges[max_plateau - 1]  = 1;
                    bridges[max_plateau]  = 1;
                }
                max_plateau = count;
                fclose(input); 
            }
        }

        [...]

        // The same fix for the output file

        if (argv[argc - 1] == NULL) {
            return 2;
        } else {
            int odd = 0;
            find_result(&ungerade, anzahl, &bruecken);
            FILE *output = fopen(argv[argc - 1], "w");
            if (output == NULL) {
                free(bridges);  // OK to call free with a null pointer
                return 2;
            } else {
                print(output, odd, &bridges, count);
                fclose(output);
                // should we free bridges?
            }
        }

CodePudding user response：

Check the value returned by fopen:

FILE *input;
if( (input = fopen(argv[i], "r")) == NULL ){
    perror(argv[i]);
    return 2;
}

ferror is only used to check the error indicator on a file handle, which can be set if some operation on the handle fails. Failing to open the file is not an operation on the file handle. The file handle does not exist until fopen successfully creates it. If the path does not exist (or any other condition prevents fopen from succeeding), fopen will return NULL. It is invalid to pass NULL as an argument to ferror.

CodePudding user response：

fopen() returns NULL when it failes, so you must check for it before passing the return value to ferror().

In other words, you should use

if(input == NULL || ferror(input))

instead of

if(ferror(input))

(add check like this also for output)

CodePudding user response：

Its quite simple, Please replace ferror(input) with !file while handling fopen error condition.