current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 2 down vote favorite

I thought about using a macro to auto close FILE pointers at the end of a block. My solution so far is:

FILE *fopen_safe(const char *filename, const char *mode) {
    FILE *fp = fopen(filename, mode);

    if (!fp) {
        perror("fopen");
        exit(EXIT_FAILURE);
    }

    return fp;
}

#define with_fopen(fp, filename, mode) \
    for (FILE *fp = fopen_safe(filename, mode), _invariant = { ._r = 1 }; _invariant._r--; fclose(fp))


// Usage:
with_fopen(fp, "test", "w") {
    fprintf(fp, "test\n");
}

This solution is not elegant as i am using an int in the struct to create my loop invariant.

Is there another way to auto close files? Can a block be associated with a macro in a smarter way?

share|improve this question
    
Is there another way to auto close files? Port your code to C++ :-) C really can't do this in any reasonable way. Just a fact of life. – Nate Eldredge Mar 26 at 4:26
    
Use function pointers along with a void * to fake closures. Not nice, but works unless you're using longjmp. – Daniel Jour Mar 26 at 6:24
up vote 4 down vote accepted

There are few problems with this macro.

  • It relies on an undocumented layout of FILE structure
  • It cannot be nested (try it with -Wshadow)
  • It assumes that the block is always left through the end

  • It may create suprises:

        for (....) {
        with_fopen(fp, ...) {
            int rc = do_something(fp);
            if (rc == -1) {
                break;
            }
        }
    }

    The programmer (with or without Python background) expects the outer loop to be broken. Instead the loop hidden inside with is.

Nice try however.

share|improve this answer
up vote 3 down vote

Unfortunately, C just isn't designed for this kind of thing. Or more accurately, C can do this perfectly fine, but not within what is commonly called "structured programming."

In C++, the correct answer would be RAII. In Java, it would be try-with-resources. In C, we have none of those things. Instead, the least bad way to do cleanup code in C is with goto:

int func(/* arguments */) {
    int rc = 0;
    char *buf = NULL;
    FILE *fp = fopen(/*...*/);
    if(!fp){
        rc=errno;
        goto err_fp;
    }
    buf = malloc(/*...*/);
    if(!buf){
        rc=errno;
        goto err_buf;
    }
    /* Put some code here
     * In this block, don't write return retval;
     * Instead, write rc=retval; goto out;
     */
out:
    free(buf);
err_buf:
    fclose(fp);
    /* fclose() error checking elided for simplicity.
     * You have to call it in a loop to handle EINTR correctly.
     */
err_fp:
    return rc;
}

This ensures that buf is only freed if it was successfully malloc'd,* fp is only closed if it was opened, and all resources are cleaned up on every code path.

* free(NULL) is legal and harmless. In real code, you should take advantage of this (e.g. by merging the err_buf label into the out label). I'm only using malloc/free as an example to show how this pattern nests.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.