12

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?

Nimantha
  • 6,405
  • 6
  • 28
  • 69
user3437503
  • 159
  • 1
  • 1
  • 4

5 Answers5

8

That's normal, when you call fclose(fp1) when fp1 is NULL.

BTW

fopen("users.sav","w");

is useless because you don't assign the return value to a file pointer. That means the users.sav file will be opened for writing, but you will never be able to write anything in it .

Jabberwocky
  • 48,281
  • 17
  • 65
  • 115
4

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.

Engineer2021
  • 3,288
  • 6
  • 29
  • 51
2

I'm not really sure what you're trying to do, but the immediate problem is here:

if(fp1==NULL)
   fclose(fp1);

After asserting that fp1 is NULL, you're trying to call close on the null pointer, which will cause a segmentation fault.

If all you want to do is verify that the file exists, try something like What's the best way to check if a file exists in C? (cross platform)

Community
  • 1
  • 1
mtripp100
  • 231
  • 1
  • 5
0

The man page of fclose says -

The behaviour of fclose() is undefined if the stream parameter is an illegal pointer, or is a descriptor already passed to a previous invocation of fclose().

The error is in the if block in your code.

if(fp1==NULL)
{
    fopen("users.sav","w");
    fclose(fp1);  // passing NULL to fclose invokes undefined behaviour
}   
ajay
  • 9,402
  • 8
  • 44
  • 71
0

Another unrelated problem:

This line is probably not what you want:

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

If we write it correctly formatted the error becomes obvious:

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

exit(1);
return 0 ;

Actually we will get to exit(1) even if checkfile is zero.

You probably want this:

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

return 0 ;

Conclusion: format your code correctly and many errors will suddenly look obvious.

Jabberwocky
  • 48,281
  • 17
  • 65
  • 115
  • You should probably roll this into your other answer, good catch :) – Engineer2021 Mar 19 '14 at 13:10
  • @GIJoe: Well, I was not sure if I should have done this because this issue it's totally unrelated to the original question. – Jabberwocky Mar 19 '14 at 13:11
  • I would.. makes your answer stronger. – Engineer2021 Mar 19 '14 at 13:13
  • It definitely did... It also pointed out a whole lot of other complications in my code that I didn't notice (hence it was not included in my question) but would probably haunt me as well... I guess this **final project** thing is just really getting on my nerves. – user3437503 Mar 23 '14 at 07:27