Click here to Skip to main content
15,881,839 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
C#
#include<stdio.h>
#include "file.h"
void openfilechk()
{
    char ch;
    fp = fopen("abc.txt","r");
    while( ( ch = fgetc(fp) ) != EOF )
    {
      printf("%c",ch);
    }
//    fclose(fp);
    printf("file readed \n");
    if (fclose(fp) == 0)
    {
                       //close was successful
    //    fp = 0;     // this ensures you the file is closed
        printf("\nFILE SUCCESSFULLY CLOSED\n");
    }
}
int main()
{
    openfilechk();
    if (fp == NULL)
    {
                       //close was successful
    //    fp = 0;     // this ensures you the file is closed
        printf("\nFILE SUCCESSFULLY CLOSED\n");
    }
    return 0;
}


portion inside main is not validating whether a file is closed?

file.h

FILE *fp;
Posted
Comments
Marius Bancila 23-Jan-13 2:51am    
What is openfilechk() supposed to do? If you're not setting fp to NULL, what's the point of checking it later in main(). You need to explain what you want to do.

1 solution

You openfilechk function does not change fp to indicate the closed status. As a result, if it opened sucessfully, it always leaves it indicating the file block you opened.
C#
#include<stdio.h>
#include "file.h"
void openfilechk()
{
    char ch;
    fp = fopen("abc.txt","r");
    while( ( ch = fgetc(fp) ) != EOF )
    {
      printf("%c",ch);
    }
    printf("file readed \n");
    if (fclose(fp) == 0)
    {
                       //close was successful
        fp = NULL;     // this ensures you the file is closed
        printf("\nFILE SUCCESSFULLY CLOSED\n");
    }
}
int main()
{
    fp = NULL;         // Give it an initial value
    openfilechk();
    if (fp == NULL)
    {
                       //close was successful
        printf("\nFILE SUCCESSFULLY CLOSED\n");
    }
    return 0;
}

BTW: It's a style thing, but I wouldn't declare variables in a ".h" file - constants, and structure definitions only.
I'd also return fp from the openfilechk method and store it in a local variable rather than using a global anyway - that way you can use it for two different files in the future. In the case of a function defined to check if it was possible to open teh file, I'd return a bool rather than a FILE* anyway - it makes your code a lot more readable.
 
Share this answer
 
Comments
nv3 23-Jan-13 4:13am    
Very well analyzed.
mvigsnhwar 23-Jan-13 4:50am    
one general question whether it is necessary to put fp = NULL after fclose(fp);
OriginalGriff 23-Jan-13 5:03am    
Yes - the definition of fclose does not state that it modifies the parameter in any way, (and it's a value parameter anyway, so it can't be changed) so it will still contain a pointer to a FILE object after the call - the FILE object itself may be invalid but the pointer probably isn't. If you don't reset it to NULL, then it will fail your test every time.

Do consider keeping the FILE* internal to your function and returning a boolean value instead - it does make your code more readable, and it "hides" the implementation detail from code that doesn't really need to know it.
Marius Bancila 23-Jan-13 16:11pm    
The answer is yes, but only if you need it afterwards.

However, your openfilechk() looks like a function that should return a bool, rather than setting a global variable that you check afterwards. As I already mentioned earlier, you're telling us to little of what you want to do.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900