Preventing the closing of an already closed file descriptor/pointer

Like the title said, I'm wondering how one would avoid doing that. The man pages mentioned that fclose()-ing and already closed file descriptor would result in "undefined behavior", which I'm taking as "it might or might not crash your program". Since I would very much like to not have my programs crash I've been trying to figure out a practical solution to this problem. Things I have tried (which worked well, but seems sloppy to me, and it really only worked when I have only one pointer/file descriptor) is using a flag that's set to one if the file is opened and zero if it's closed. Something like this:
[code]
FILE *open_file( char *fname, int *flag )
{
FILE *fd;
if ( *flag == 0 ) { /* Is the file already open? */
if ( ( fd = fopen( fname, "r+" ) ) == 0 ) { /* Check for errors. */
perror( "Couldn't open file." );
exit( -1 );
}

*flag = 1;
}
else
puts( The file descriptor is already open, close it first." );

return fd;
}

void close_file( FILE *fd, int *flag )
{
if ( *flag == 1 ) { /* Is the file descriptor full of openness? */
fclose( fd ); /* If so, make it full of closedness. ^_^ */
*flag = 0;
}
}
[/code]
However as you can see it's not scalable. :( Also just freeing it at the end of the program isn't very practical either since one might open and close a given pointer or file descriptor multiple times in a program. So yeah, would anybody happen to know of an elegant and practical solution to this?

Oooh, also, this is not entirely related, but I've been having some problems with fflush(), for some reason it just doesn't seem to do anything. :/ For example given the code:
[code]
scanf( '%s', string )
fflush( stdin );
getchar();
[/code]
The getchar() will _still_ catch a new line left over in stdin. I'm kinda sick of this problem since I have _no_ clue why this is happening. Could anybody help me with this also please? For possible relevance and completeness I'm running Gentoo and using GCC with the 2.6.22 kernel. ^_^ (just in case that's important :) )

Comments

  • I make it a matter of programming style. I use a technique I call "programming from both ends" where I take advantage of the editor's ability to insert code between existing parts. For example, I always start a program with
    [code]
    main()
    {
    }
    [/code]
    This assures that the opening brace has a closing brace. I then insert the code between the existing braces.

    I do the same thing opening and closing files. E.g.
    [code]
    main()
    {
    yatta
    yatta
    yatta

    f1 = fopen(yatta, yatta) ;
    yatta
    yatta
    fclose (f1) ;
    }
    [/code]
    Note that I indent code that comes between the opening and closing of the file. The indentation reminds me not to try to open the file in this section.

    Sometimes I open/close a file withing a loop, which means I have to be careful if i use [b]break[/b].
    [code]
    main()
    {
    yatta
    yatta
    yatta

    while (whatever) {
    yatta
    yatta

    f1 = fopen(yatta, yatta) ;
    yatta
    yatta

    if (boolean) {
    fclose (f1) ; // jumping out of the open/close pair
    break ;
    }
    fclose (f1) ;

    yatta
    yatta
    }
    }
    [/code]
    It's no panacea but it works for me, especially if I keep the program modular.

  • Hmm, that's not a bad idea, but it's still a little clumsy methinks. Especially if you have a program where the file descriptor may or may not point to a file at any given point where you happen to do something else with it, for instance if you had code that looked something like this:
    [code]
    for( ;; ) {
    switch( something ) {
    case 1:
    open_file();
    break;
    case 2:
    close_file();
    break;
    }
    }
    [/code]
    I guess that's a really silly example, but one really can't have a fclose() for every fopen() in that case since things may happen in any order really. And avoiding that just to keep a nice fopen() fclose() sequence would dramatically cut down on the flexibility of a program methinks.
  • : Hmm, that's not a bad idea, but it's still a little clumsy methinks.
    [blue]
    Actually, I think it's rather elegant. If I could design my own programming language, I'd design [b]open()[/b] thus..
    [/blue]
    [code]
    open (f, ...) {
    yatta
    yatta
    }
    [/code]
    [blue]
    so that the file is open only between the braces. When the code leaves the area between the braces, either by falling off the bottom, a [b]break[/b] or a [b]return[/b], the file is closed. There would be no [b]close[/b]. My method emulates this.
    [/blue]
    : Especially if you have a program where the file descriptor may or
    : may not point to a file at any given point where you happen to do
    : something else with it, for instance if you had code that looked
    : something like this:
    : [code]:
    : for( ;; ) {
    : switch( something ) {
    : case 1:
    : open_file();
    : break;
    : case 2:
    : close_file();
    : break;
    : }
    : }
    : [/code]:
    : I guess that's a really silly example, but one really can't have a
    : fclose() for every fopen() in that case since things may happen in
    : any order really.
    [blue]
    Things shouldn't happen "in any order". If it does you, the programmer, are not in control. The code you posted seems to be an example of the programmer not in control.
    [/blue]
    : And avoiding that just to keep a nice fopen()
    : fclose() sequence would dramatically cut down on the flexibility of
    : a program methinks.
    :
    [blue]
    I'm of the opposite opinion. People advanced the same argument when Djikstra first suggested that [b]goto[/b] be avoided.

    I'd suggest you read some books on style. [italic]The Elements of Programming Style[/italic] and [italic]Software Tools[/italic] by Kernighan & Plauger are good, albeit somewhat dated since they are written in Fortran. More modern tomes include [italic]The Practice of Programming[/italic] by Kernighan & Pike, and [italic]Code Complete[/italic] by Steve McConnell.
    [/blue]
  • Actor's advice to indent is good for all kind of resources that you open and close, not only files.

    Anyway, I have a vauge memory that it is safe to pass a NULL pointer to fclose(), but I can't find anything in the C standard about it.

    The best way might be to wrap in fopen() and fclose() in a safe code module, like this (not tested):

    [code]/* h file */
    typedef struct
    {
    FILE* fp;
    BOOL is_open;
    } FileOpener;


    void file_init (FileOpener* fo);

    BOOL file_open (FileOpener* fo,
    const char* path,
    const char* mode);

    void file_close (FileOpener* fo);
    [/code]

    [code] /* c file */

    void file_init (FileOpener* fo)
    {
    fo->fp = NULL;
    fo->is_open = FALSE;
    }

    BOOL file_open (FileOpener* fo,
    const char* path,
    const char* mode)
    {
    if(fo->is_open)
    {
    fclose(fo->fp);
    }

    fo->fp = fopen(path, mode);

    if(fo->fp == NULL)
    {
    fo->is_open = FALSE;
    }
    else
    {
    fo->is_open = TRUE;
    }

    return fo->is_open;
    }

    void file_close (FileOpener* fo)
    {
    if(fo->is_open)
    {
    fclose(fo->fp);
    fo->is_open = FALSE;
    }
    }[/code]

    [code]int main()
    {
    FileOpener my_file;

    file_init(&my_file);

    switch(something)
    {
    case this:
    file_open(&my_file, "somefile.txt", "r");
    break;

    case that:
    file_close(&my_file);
    break;
    }
    }


    [/code]
  • @Lundin That's actually not a bad idea, I really hadn't thought about using a struct for this. It would really solve the messiness of having separate flags. Oh, and I did some reading and it seems like you can pass a NULL to free(), (which solves my problem with the pointers kinda) however it doesn't seem like fclose() works like that.


    :Things shouldn't happen "in any order". If it does you, the programmer, are not in
    :control. The code you posted seems to be an example of the programmer not in
    :control.
    I _think_ I get what you're saying, though on the other hand I'm really not. Could you please perhaps explain what you mean?

    I mean I can understand that things should _not_ happen in a why which you haven't actually planned. However what about choosing something from a menu? The person can pick anything after anything else, so in essence things are happening in any order. In the underlying code you can make sure certain things to happen before other things, but the order in which whoever uses the program does things should still be able to happen in an unspecified order shouldn't it?
    Sorry if all that is bleedingly obvious, but it's apparently not for me and I'm really just trying to understand what you mean. ^_^

    Oh also, why not check for an open file descriptor before one does something with it and check if it's closed when one leaves the function?

    [EDIT]: Wow this forum thingy is confusing. :/ I kinda expected that my message would be placed at the end of the thread.
  • : Oh, and I did some reading and it seems
    : like you can pass a NULL to free(), (which solves my problem with
    : the pointers kinda) however it doesn't seem like fclose() works like
    : that.

    Before I looked it up in the standard, I was pretty sure it worked in that way too. However, the standard doesn't mention at all what will happen. This means that passing NULL to fclose() is undefined behavior and may cause your program to crash. Several topics on the net suggest the same.

    I think we have it mixed it up with free() function, which works perfectly fine in case you pass a NULL pointer to it.



    : I mean I can understand that things should _not_ happen in a why
    : which you haven't actually planned. However what about choosing
    : something from a menu? The person can pick anything after anything
    : else, so in essence things are happening in any order.

    The common way to handle this in a GUI app is to disable certain menu items during certain conditions, so that they show up as "grey" to the user. If you don't disable the menu options, yeah then the user might go bananas and close files before having opened one and other such dumb things users tend to do.


    : Oh also, why not check for an open file descriptor before one does
    : something with it and check if it's closed when one leaves the
    : function?

    That might work with Win API functions like ReadFile() etc, but I don't think you can do it with pure standard C.


    : [EDIT]: Wow this forum thingy is confusing. :/ I kinda expected that
    : my message would be placed at the end of the thread.

    It depends on which post you reply to. You replied to Actor's post, so the reply ends up after his post.
  • :: Oh, and I did some reading and it seems
    :: like you can pass a NULL to free(), (which solves my problem with
    :: the pointers kinda) however it doesn't seem like fclose() works like
    :: that.
    : Before I looked it up in the standard, I was pretty sure it worked in that way
    : too. However, the standard doesn't mention at all what will happen. This means
    : that passing NULL to fclose() is undefined behavior and may cause your program to
    : crash. Several topics on the net suggest the same.

    Umm, not to be rude, but I think you kinda misread what I said. We're saying the same thing there which is: passing NULL toe _free()_ doesn't cause problems, but doing the same to _fclose()_ does. ^_^

    : The common way to handle this in a GUI app is to disable certain menu items
    : during certain conditions

    Yeah but I really don't actually do all that much GUI programming, (read: "I don't do GUI programming _at all_) I'm more of a console girl. ^_^

    : If you don't disable the menu options, yeah then the user might go bananas and
    : close files before having opened one and other such dumb things users tend to
    : do.

    Hmm, what about just writing your code in a why that it does matter if the user attempts that? I mean, wouldn't that make for more stable code in more situations?

    I really have no idea what the Win API functions says, most of the things that I do tend to be with standard C. I've used inotify once though, but that's about as far as I've strayed from standard C.

    : It depends on which post you reply to. You replied to Actor's post, so the
    : reply ends up after his post.

    That is confusing, I think it would have been _much_ easier if it just followed a normal forum structure. :/
  • [blue]
    : I mean I can understand that things should _not_ happen in a why
    : which you haven't actually planned. However what about choosing
    : something from a menu? The person can pick anything after anything
    : else, so in essence things are happening in any order. In the
    : underlying code you can make sure certain things to happen before
    : other things, but the order in which whoever uses the program does
    : things should still be able to happen in an unspecified order
    : shouldn't it?
    [/blue]
    A menu is a good example for explaining my thesis. Menu driven programs generally take the form of a loop.
    [code]
    // pseudocode

    repeat
    read option
    switch option
    case 0:
    end program
    case 1:
    do something
    case 2:
    do something else
    [/code]
    If such a program uses a file there are three possibilities. The first possibility is that the program uses only one file which the user need not specify. In that case...
    [code]
    // pseudocode

    open file
    flag = TRUE
    while flag
    read option
    switch option
    case 0: // terminate program
    flag = FALSE
    case 1:
    do something
    case 2:
    do something else
    close file
    [/code]
    This solution keeps the opening and closing of the file out of the hands of the user and makes the program easier to use.

    The second case is where the program still uses only one file but the user needs to specify which file. In that case the best solution is a console program where the user passes the name of the file to the program via the command line, i.e., command line arguments.
    [code]
    // pseudocode
    if number_of_arguments > 0
    associate file with argument[1]
    open file
    flag = TRUE
    while flag
    read option
    switch option
    case 0: // terminate program
    flag = FALSE
    case 1:
    do something
    case 2:
    do something else
    close file
    else
    quit
    [/code]

    If command line arguments are not an option the best solution is a menu nested within a menu.
    [code]
    main program
    // pseudocode

    repeat
    read option
    switch option
    case 0:
    end program
    case 1:
    prompt for and select file
    open file
    sub_menu
    close file

    function sub_menu
    repeat
    read option2
    switch option2
    case 0:
    return
    case 1:
    do something
    case 2:
    do something else
    [/code]
    The user gets to choose what file he wants to use but he is not burdened with the responsibility of opening and closing the file. And once he is done with one file he can select another.

    A third possibility is a case where the program works with more than one file at a time (e.g., a relational database), but I won't go into that.

    The problem with a program such as this
    [code]
    // pseudocode

    repeat
    read option
    switch option
    case 0:
    end program
    case 1:
    specify file
    case 2:
    open file
    case 3:
    close file
    case 4:
    do something
    case 5:
    do something else
    [/code]
    is that you get involved in assuring that things are not done out of order, which means a lot of flags, or state tables or other junk that increases the complexity. Unfortunately, that seems to be the way programs are written today (witness buttons, boxes, etc that are "grayed out") If you are working with console program I suggest you try to avoid that.

  • I actually got to this problem as a result of the the last example you've shown. I'm working on a self study course thingy and I'm doing file IO at the moment, and one of the exercises is to make a kind of database program. For which I actually started implementing something similar to your last example. (just with an optional command line file specification)

    Thanks a mil for your help! I'm going to see about implementing the sub menu idea, and I'll keep a closer eye out for things like this and try to avoid adding unnecessary stuff. ^_^

    Oooh just one last thing. Could someone perhaps just tell me how I could figure out why [b]fflush()[/b] doesn't want to work? I try to flush stdin, but if I run [b]getchar()[/b] I _still_ get stuff from it. :/ It's like although I run [b]fflush()[/b] it's still not doing anything and I _really_ have no idea how to figure out why that is.
Sign In or Register to comment.

Howdy, Stranger!

It looks like you're new here. If you want to get involved, click one of these buttons!

Categories