CODEWITHSUNDEEP
cdebugging
CodeEnjoyer
CodeEnjoyer

Reputation: 39

fopen() fails but seems to not return NULL?

I have got the following code:

#include <stdio.h>
#include "config.h"
#include <errno.h>

char buffer[50];
long long bufSize = 50;
FILE* fptr;

char* readConfig(char* buffer, size_t bufSize) {
    fptr = fopen("config.txt", "rt");
    if (fptr == NULL) {
        return "error opening config file: %s", strerror(errno);
    } else {
        if ((fgets(buffer, bufSize, fptr)) == NULL) {
            fclose(fptr);
            return "error reading config file: %s", strerror(errno);
        }
        else {
            fclose(fptr);
            return buffer;
        }
    }
}

For test purposes I deleted the config.txt file so that the fopen() function should return NULL. What wonders me is that fopen("config.txt", "rt"); fails, but when debugging the code it just skips the "if (fptr == NULL) {...}" part and directly jumps out of the function.

When going on with the debugging process, I get the following error when trying to work with the return Value of readConfig() "0xC0000005: Access violation reading location 0xFFFFFFFFA4E0EB70"

Upvotes: 0

Views: 171

Answers (1)

user9706
user9706

Reputation:

  1. Cannot compile your code as you shared a snippet (no main()) and didn't include config.h.
  2. #include <string.h> for strerror().
  3. Suggest caller passes in local variables instead of global variables.
  4. Instead of hard-coding the size in both buffer[50] and bufSize = 50; use sizeof to determine the size of the array. The other good alternative is to define a constant.
  5. The fopen() mode "t" isn't standard, so either leave it out or tag your program with windows or whatever.
  6. As you return on error, eliminate the unnecessary else & indentation.
  7. The expression return "error opening config file: %s", strerror(errno); doesn't work the way you expect, it will evaluate the first part in void context then return the 2nd part strerror(errno). I was not able to otherwise reproduce any ill effects.
  8. fgets() return NULL on eof or error but not appear to set errno. You can use feof() and/or ferror() to tell which if needed.
  9. After the call to fgets() you call fclose() prior to inspecting errno, so it have the status of the fclose() call instead.
  10. It's a bad design to return either error message or the value you read from the file as you cannot tell them apart. Changed to return NULL on success.
#include <errno.h>
#include <stdio.h>
#include <string.h>

char *readConfig(char *buffer, size_t bufSize) {
    FILE* fptr = fopen("config.txt", "r");
    if(!fptr)
        return strerror(errno);
    if(!fgets(buffer, bufSize, fptr)) {
        fclose(fptr);
        return "fgets eof/error";
    }
    fclose(fptr);
    return NULL;
}

int main(void) {
    char b[50];
    const char *error = readConfig(b, sizeof b);
    if(error) {
        printf("error: %s\n", error);
        return 1;
    }
    printf("%s", b);
}

Consider having caller open the file and pass in the FILE *. It gives you the flexibility, for instance, to use stdin as the file handle.

I prefer using goto instead of the multiple returns when resource cleanup is required. It's a bit more verbose here but each error case is handled the same way. When you swap the arguments you can document how they are related with recent compilers:

char *readConfig(size_t bufSize, char buffer[bufSize]) {
    char *r = NULL;
    FILE* fptr = fopen("config.txt", "r");
    if(!fptr) {
        r = strerror(errno);
        goto out;
    }
    if(!fgets(buffer, bufSize, fptr)) {
        r = "fgets eof/error";
        goto out;
    }
out:
    fptr && fclose(fptr);
    return r;
}

Upvotes: 1

Related Questions