fclose() causes Segmentation Fault

Question

I've been trying simple file handling in C and I wanted to make sure that the file can be accessed tried using this

#include<stdio.h>

main()
{
    CheckFile();
}

int CheckFile()
{
    int checkfile=0;

    FILE *fp1;
    fp1 = fopen("users.sav","r");

    if(fp1==NULL)
    {
        fopen("users.sav","w");
        fclose(fp1);
    }   
    if(checkfile!=0)printf("\nERROR ACCESSING FILE!\nNow exiting program with exit code: %d\n",checkfile);exit(1);
    return 0;
}

then it displays

Segmentation fault (core dumped)

but it doesn't segfault if the file already exists beforehand (e.g. when i created it manually or when i run the program the second time)

Please help. I need this for our final project due in a week and I haven't gotten the hang of files and pointers yet.

I'm using "gcc (Ubuntu/Linaro 4.8.1-10ubuntu9) 4.8.1"

P.s

I saw this in another question

There's no guarantee in your original code that the fopen is actually working, in which case it will return NULL and the fclose will not be defined behaviour.

So how exactly do I check if it worked?

Answer 1

fopen returns a FILE pointer. It will return NULL and set the global errno to indicate the error. If you want to check the errno, you have to check if after you check if fopen returned NULL.

if (fp1 == NULL)
{
    printf("fopen failed, errno = %d\n", errno);
}

Otherwise, you may get an errno from something else, not necessarily your fopen call. Also include errno.h. You also don't need to call fopen("users.sav","w"); again. You aren't reassigning the pointer nor checking it again.

I don't see a reason to call fclose here since if fopen returns NULL, there isn't anything to close. That is probably the reason for your seg fault. You are trying to close a null pointer. More information on fopen failures.

Another comment on your code. If you are going to return an int from CheckFile, it should probably not be 0 on fail. I would return -1 to indicate an error. Better yet, you could return the global errno. Also, main should be int main() and you should return 0; at the end. I don't particularly care for your naming scheme of CheckFile. In C, check_file or camelCase of checkFile would be better.

In CheckFile, your one line if statement could be formatted and work more properly if you formatted it on multiple lines. It doesn't do what you think it does currently:

if(checkfile!=0)
{
   printf("\nERROR ACCESSING FILE!\nNow exiting program with exit code: %d\n", checkfile);
   exit(1);
}

Also, checkfile is never set anywhere in your code.. other than zero. So the code in the if statement will not execute, period.