How to handle error in C?

Question

Say, I have code like this:

int foo() {
    ...
    int buff_size;
    scanf("%d", &buff_size);
    FILE * fp = fopen("./file", "a+");
    char *buff = malloc(buff_size*sizeof(char));
    char *buff2 = malloc(buff_size*sizeof(char));
    char *buff3 = malloc(buff_size*sizeof(char));
    while (!feof(fp)) {
        /*do something, say read, write etc.*/
        if (/*error case 1*/) {
            free(buff);
            free(buff1);
            free(buff3);
            fclose(fp);
            return -1;
        }
        else if (/*error case 2*/) {
            free(buff);
            free(buff1);
            free(buff3);
            fclose(fp);
            return -2;
        }
        ...
        else if (/*error case k*/) {
            free(buff);
            free(buff1);
            free(buff3);
            fclose(fp);
            return -k;
        }
    }
    ...
    free(buff);
    free(buff1);
    free(buff2);
    fclose(fp);
    return 0;
    }

for C doesn't provide try...throw...finally syntax, I have to close file descriptors and free heap memories pointer that I created before I return a error integer code. It produces some duplicated code that makes the code ugly.

Do anyone know how should I modifly this kind of code to make it looks more brief?

If you always allocate the same fixed size, why not use arrays? Also please read [Why is “while ( !feof (file) )” always wrong?](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) — Some programmer dude, Apr 22 '17 at 04:11
Why not change `char *buff = malloc(1024*sizeof(char));` to `char buff[1024];` (ditto with the others)? — Ed Heal, Apr 22 '17 at 04:22
`c++` tag should be removed from the question. Coz, answer will different in `c++` than in `c`. — army007, Apr 22 '17 at 04:37
It just a example. For many case, the buffer's size is determined by the input, so array doesn't works unless I use C99 standard. — Stephen.W, Apr 22 '17 at 04:54
Good discussion of "try catch" in C: http://stackoverflow.com/questions/10586003/try-catch-statements-in-c — TonyB, Apr 22 '17 at 04:54
Never use `while (!feof(fp)) {` The function: `feof()` does not do what you think it does. — user3629249, Apr 23 '17 at 18:28
when calling any of the heap allocation functions (malloc, calloc, realloc) 1) always check (!=NULL) the returned value to assure the operation was successful 2) the expression: `sizeof(char)` is defined in the standard as 1, multiplying by 1 has absolutely no effect and just clutters the code, making it more difficult to understand, debug, etc. Suggest removing that expression. — user3629249, Apr 23 '17 at 18:29
when calling `fopen()`, always check (!=NULL) the returned value to assure the operation was successful — user3629249, Apr 23 '17 at 18:30
when calling any of the `scanf()` family of functions: always check the returned value to assure the operation was successful. — user3629249, Apr 23 '17 at 18:32

score 4 · Answer 1 · edited May 23 '17 at 12:09

Your code, as written is equivalent to

    FILE * fp = fopen("./file", "a+");
    int return_value = 0;                  /*  assume zero means no error */
    char *buff = malloc(1024*sizeof(char));
    char *buff2 = malloc(1024*sizeof(char));
    char *buff3 = malloc(1024*sizeof(char));

    while (feof(fp))
    {
        /*do something, say read, write etc.*/
        if (/*error case 1*/)
        {
            return_value = -1;
        }
        else if (/*error case 2*/)
        {
            return_value = -2;
        }
        // ...
        else if (/*error case k*/)
        {
            return_value = -k;
        }
    }

    if (return_value == 0)   /* if no error has occurred, we can still do stuff  */
    {
        ...
    }

    /*   I have assumed all buffers need to be released 
         and files closed as the function returns
    */

    free(buff);
    free(buff1);     /*  you probably intend buff2 and buff3 here, to match the malloc() calls */
    free(buff2);
    fclose(fp);
    return return_value;
}

I have not, however, addressed two critical errors in your code. You need to.

fopen() can fail, and return NULL. If it does, passing it to feof() or to fclose() gives undefined behaviour. Your code is not checking that at all, and needs to both before call of feof() and fclose().
A loop of the form while (!feof(fp)) {read_something_from(fp); use_something();} is a bad idea. Have a look at Why is “while ( !feof (file) )” always wrong? for more info.

Less critical - for fixed size arrays (size fixed at compile time) you probably don't need to use malloc() which also means you don't need to use free(). However, you still need to deal with potential failures of fopen().

@alk - I did not suggest avoiding a `while` loop. I said that `while(!feof())` (specifically) is a bad idea. — Peter, Apr 22 '17 at 09:15

score 1 · Answer 2 · 2017-04-22T04:42:46.267

1

For this sort of thing with common cleanup code, I'd use a return value variable and a goto:

int foo() {
    ...
    // append extended mode ... what are you doing with this?
    FILE * fp = fopen("./file", "a+");
    char buff[1024];
    char buff2[1024];
    char buff3[1024];

    int ret = 0;

    while (/* file I/O is valid on fp */) {
        /*do something, say read, write etc.*/
        if (/*error case 1*/) {
            ret = -1;
            goto cleanup;
        }
        else if (/*error case 2*/) {
            ret = -2;
            goto cleanup;
        }
        ...
        else if (/*error case k*/) {
            ret = -k;
            goto cleanup;
        }
    }
    ...
cleanup:
    free(buff);
    free(buff2);
    free(buff3);
    fclose(fp);
    return ret;
}

This also prevents having to copy-paste code, or retyping with errors (as your current code has).

edited Apr 22 '17 at 04:42

answered Apr 22 '17 at 04:18

Please read the comment above about the use of `feof` in `while` looks – Ed Heal Apr 22 '17 at 04:21
Edited. Now OP should choose their "is valid" expression. – Apr 22 '17 at 04:44
using `goto()` is a horrible idea. the result is 'spaghetti' code. Much better to write a 'sub' function that handles the cleanup and pass it any parameter values that it needs. – user3629249 Apr 23 '17 at 18:35

How to handle error in C?

2 Answers2