-2

If I open a file handle, but the function encounters other error which is not file error, before closing the handle, would the file handle be leaked?

FILE* fp = fopen("test.txt", "r");
if (other_condition) {
    perror("other error occurred!");
    return EXIT_FAILURE;
}

How to use C to implement RAII template?

  • Bad example - function returns because you couldn't get the resource so there is nothing to leak. – John3136 Nov 16 '17 at 03:19
  • @John3136 change now. –  Nov 16 '17 at 03:20
  • umm how about a goto – asio_guy Nov 16 '17 at 03:22
  • @potato that would be bad code style? a function contains process and error handle –  Nov 16 '17 at 03:24
  • There is no automatic RAII idiom in C; is is an option with C++. There are no automatic destructors in C. You have to write the code. You didn’t show an error check for the `fooen()`; that’s bad style. You don’t show the `fclose()` before successful return. You need an `fclose()` on the error return. – Jonathan Leffler Nov 16 '17 at 03:40
  • @JonathanLeffler I mean that , after open file , other error occurs. –  Nov 16 '17 at 06:48

3 Answers3

1

For a single resource like a file handle, consider:

int rc = EXIT_FAILURE;
FILE *fp = fopen(file, "r");
if (fp != NULL)
{
    rc = function_using_resource(fp /* , … */);
    fclose(fp);
}
else
{
    perror(file);
}
return rc;

This only uses the resource if it was successfully allocated, and returns the status from the function that uses the allocated resource. This function opened the file; it is responsible for closing it. It does close it if it was opened.

The perceived downside to this is that you end up with a lot of functions which are individually not very big. However, extending this to multiple resources is hard. You probably end up using a goto, but it is a carefully structured use. For example, consider code that opens a file stream, and allocates two blocks of memory. You might end up with code like:

int function_example(int num_items)
{
    int rc = EXIT_FAILURE;
    struct WotNot *wotsit = NULL;
    struct Havoc  *wreaker = NULL;
    FILE *fp = fopen(file, "r");

    if (fp == NULL)
    {
        err_remark("failed to open file '%s' for reading\n", file);
        goto cleanup;
    }

    if ((wotsit = malloc(num_items * sizeof(*wotsit))) == NULL)
    {
        err_remark("failed to allocated %zu bytes of memory\n",
                   num_items * sizeof(*wotsit));
        goto cleanup;
    }

    if ((rc = function_using_file(fp, num_items, wotsit)) != EXIT_SUCCESS)
    {
        err_remark("processing of wotsits failed\n");
        goto cleanup;
    }

    if ((wreaker = malloc(num_items * 2 * sizeof(*wreaker)) == NULL)
    {
        err_remark("failed to allocated %zu bytes of memory\n",
                   num_items * 2 * sizeof(*wreaker));
        goto cleanup;
    }

    if ((rc = function_wreaking_havoc(num_items, wotsit, wreaker) != EXIT_SUCCESS)
    {
        err_remark("failed to wreak appropriate havoc\n");
        goto cleanup;
    }

    if ((rc = other_function(fp, num_items * 2, wreaker)) != EXIT_SUCCESS)
    {
        err_remark("failed to process the wotsits — not enough havoc?\n");
        goto cleanup;  // Systematic, but nominally superfluous
    }

cleanup:
    free(wreaker);
    free(wotsit);
    if (fp != NULL)
        fclose(fp);
    return rc;
}

Note how the resources are initialized to a harmless state (null pointers). If there's a problem, the code jumps to the cleanup block. The free() function accepts a null pointer gracefully and does nothing, so those calls don't need a check, but fclose() does not necessarily work cleanly given a null pointer, so the file stream is checked before calling fclose().

This is a fairly common idiom. Variations include multiple labels for different amounts of cleanup work. This still tends to be clearest when you don't put too much activity in this function in place of the function calls shown doing the work. However, it is not uncommon to see work done in the function other than resource allocation — the computation part of the workload is often included, especially if there's work involved such as dynamically growing one or more of the allocated arrays. It still isn't a particularly good idea to mix things up like that, but that doesn't stop people doing it anyway.

You should have a convention for whether the called function reports the errors or the calling function reports errors, or neither. You can also use various mechanisms for structuring error reporting. Making sure that handling errors is as painless as possible is important — it means people are less likely to skimp on the error reporting.

Other questions of some relevance include:

Etc.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • I mean when occurs some other error , not file problem, let program crash.at that time , has no time to close –  Nov 16 '17 at 11:59
  • If your program is crashing, you don’t get to clean up, period. And that’s true in C++ where RAII is integral to the system. Crashing means “out of control”. If you’re stopping under control, what I’ve showed is about as good as it gets, and yes, it really is that verbose. – Jonathan Leffler Nov 16 '17 at 14:16
0

In C, error checks should be manually written. (In C++, however, destructors are called automatically once an object expires.)

FILE* fp = fopen("test.txt", "r");
if (fp == NULL) { file_open_error_code; } // Recommended
if (other_condition) {
    fclose(fp); // Manually close it
    perror("other error occurred!");
    return EXIT_FAILURE;
}
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
iBug
  • 35,554
  • 7
  • 89
  • 134
0

If I open a file handle, but the function returns before closing the handle, would the file handle be leaked?

Yes.

How to use C to implement RAII template?

There's no RAII in C, and no way to implement it portably. OTOH, it's way less critical, as in C all the control flow (with the exception of longjmp) is explicit. There's no such a thing as a stack unwind triggered by an exception, your function returns only when you explicitly return, so you can (and should) clean up explicitly at all return points.

In practice, if you have a block of code that acquires resources (and thus has to clean up if something fails or at the end), it's a common pattern to have single cleanup/exit point at the end and goto to it on failure (plus sentinel values to mark what values haven't been initialized yet; another common option is to clean up strictly in reverse order of initialization and have multiple "entrypoints" to jump to depending from the amount of initialization that succeeded, but IMO it quickly becomes a mess).

int copy_file(const char *src, const char *dest) {
    int ret = -1;
    FILE *in = NULL;
    FILE *out = NULL;
    char buf[1024];
    size_t rb;
    if((in = fopen(src, "rb")) == NULL) goto cleanup;
    if((out = fopen(dest, "wb")) == NULL) goto cleanup;
    for(;;) {
        rb = fread(buf, 1, sizeof(buf), in);
        if(rb == 0) break;
        if(fwrite(buf, 1, rb, out) != rb) break;
    }
    ret = 0;
    if(ferror(in)) ret = -2;
    else if(ferror(out)) ret = -3;
cleanup:
    if(in) fclose(in);
    if(out) fclose(out);
    return ret;
} 
Matteo Italia
  • 123,740
  • 17
  • 206
  • 299