Skip to content

Instantly share code, notes, and snippets.

@julien-h
Last active December 19, 2016 19:38
Show Gist options
  • Save julien-h/06230770269343e2d23e7736fd57d3e3 to your computer and use it in GitHub Desktop.
Save julien-h/06230770269343e2d23e7736fd57d3e3 to your computer and use it in GitHub Desktop.
Comparison of different error handling patterns for the same C function

#How to handle errors and resources in C ?

Comparison of different ways to handle errors in the same C function.

The function, called db_create creates a new ImageDatabase instance.

###Table Of Content

  • Snippet 0 duplicates code
  • Snippet 1 uses nested if
  • Snippet 2 uses nested else
  • Snippet 3 uses goto
  • Snippet 4 improves on snippet 3 with two macros
  • Alternative using libraries

###Snippet 0

This version avoids deep nesting and goto but duplicates code and mixes logic with error handling.

ImageDatabase*
db_create_goto(char const* filename,
          uint32_t max_images,
          struct resolution resolutions[NB_RESOLUTIONS - 1])
{
   ImageDatabase* db = allocate_empty_db();
   if (db == NULL) {
       fprintf(stderr, "unable to allocate database");
       return NULL;
   }
   memcpy(db->resolutions, resolutions, (NB_RESOLUTIONS - 1) * sizeof(struct resolution));
   db->max_nb_images = max_images;
   if ((db->file = fopen(filename, "r+b")) == NULL) {
       fprintf(stderr, "unable to open %s", filename);
       free(db);
       return NULL;
   }
   if ((db->metadatas = calloc(max_images, sizeof(struct metadata))) == NULL) {
       fprintf(stderr, "unable to allocate metadatas");
       fclose(db->file);
       free(db);
       return NULL;
   }
   if (fwrite(db, 1, sizeof(ImageDatabase), db->file) != 1) {
       fprintf(stderr, "unable to write newly created database to %s", filename);
       free(db-metadatas);
       fclose(db-file);
       free(db);
       return NULL;
   }
   return db;
}

###Snippet 1

This version (30 lines of code) uses nested if to achieve error handling without duplication. The code uses the following structure multiple times:

  if (allocate resource and check if success) { // push on the (imaginary) stack
      <further code using the resource>
      free resource // pop the (imaginary) stack
  } else {
      signal error // unable to push further
  }
  • Upside: the nesting provides a sort of stack unwinding that ensures correct resources releasing
  • Downside: [locality cost] sometimes the else is far from the condition and this makes code hard to track down
ImageDatabase*
db_create_if(char const* filename,
             uint32_t max_images,
             struct resolution resolutions[NB_RESOLUTIONS - 1])
{
    ImageDatabase* db = allocate_empty_db();
    if (db) {
        memcpy(db->resolutions, resolutions, (NB_RESOLUTIONS - 1) * sizeof(struct resolution));
        db->max_nb_images = max_images;
        if ((db->file = fopen(filename, "r+b"))) {
            if ((db->metadatas = calloc(max_images, sizeof(struct metadata)))) {
                if (fwrite(db, 1, sizeof(ImageDatabase), db->file) == 1) {
                    return db;
                    
                    //error handling below:
                } else {
                    fprintf(stderr, "unable to write newly created database to %s", filename);
                }
                free(db->metadatas);
            } else {
                fprintf(stderr, "unable to allocate metadatas");
            }
            fclose(db->file);
        } else {
            fprintf(stderr, "unable to open %s", filename);
        }
        free(db);
    } else {
        fprintf(stderr, "unable to allocate database");
    }
    return NULL;
}

###Snippet 2 This version (30 lines of code) uses nested else to achieve error handling without duplication. else cannot be avoided in this version without duplicating the resource releasing code : it is the nesting that avoids duplication. The code uses the following structure multiple times:

if (allocate resource and check if error) {
    signal error
} else {
    <further code using the resource>
    free resource
}
  • Upsides: the else clause is short so there is no problem with code locality the nesting provides a sort of stack unwinding that ensures correct resources releasing
  • Downsides: error handling code mixed with logic, lots of nesting, hard to read
ImageDatabase*
db_create_else(char const* filename,
           uint32_t max_images,
           struct resolution resolutions[NB_RESOLUTIONS - 1])
{
    ImageDatabase* db = allocate_empty_db();
    if (db == NULL) {
        fprintf(stderr, "unable to allocate database");
    } else {
        memcpy(db->resolutions, resolutions, (NB_RESOLUTIONS - 1) * sizeof(struct resolution));
        db->max_nb_images = max_images;
        if ((db->file = fopen(filename, "r+b")) == NULL) {
            fprintf(stderr, "unable to open %s", filename);
        } else {
            if ((db->metadatas = calloc(max_images, sizeof(struct metadata))) == NULL) {
                fprintf(stderr, "unable to allocate metadatas");
            } else {
                if (fwrite(db, 1, sizeof(ImageDatabase), db->file) != 1) {
                    fprintf(stderr, "unable to write newly created database to %s", filename);
                } else {
                    return db;
                    //cleanup code below :
                }                             // fallthrough after `else`
                free(db->metadatas);
            }                                 // fallthrough after `else`
            fclose(db->file);
        }                                     // fallthrough after `else`
        free(db);
    }
    return NULL;
}

###Snippet 3 This version (34 lines of code) uses goto to achieve error handling without duplication and without deep nesting. The code has the following structure:

if (allocate resource and check for error) { // push on the (imaginary) stack
    <signal error>
    goto pop_label;
}
<further code using the resource>

pop_label: <cleanup code> // pop the (imaginary) stack

The order of the labels is reversed :

allocate resource 1                //push 1
    allocate resource 2            //push 2
       allocate resource 3         //push 3
       
       label_cleanup_resource_3    //pop 3
    label_cleanup_resource_2       //pop 2
label_cleanup_resource_1            //pop 1
  • Upsides: the stack unwinding pattern with goto is common in C and ensures correct resources releasing, link
  • Downside: some error handling code mixed with logic
ImageDatabase*
db_create_goto(char const* filename,
          uint32_t max_images,
          struct resolution resolutions[NB_RESOLUTIONS - 1])
{
    ImageDatabase* db = allocate_empty_db(); //push resource1
    if (db == NULL) {
        fprintf(stderr, "unable to allocate database");
        goto db_fail;
    }
    memcpy(db->resolutions, resolutions, (NB_RESOLUTIONS - 1) * sizeof(struct resolution));
    db->max_nb_images = max_images;
    if ((db->file = fopen(filename, "r+b")) == NULL) { //push resource2
        fprintf(stderr, "unable to open %s", filename);
        goto file_fail;
    }
    if ((db->metadatas = calloc(max_images, sizeof(struct metadata))) == NULL) { //push resource3
        fprintf(stderr, "unable to allocate metadatas");
        goto metadata_fail;
    }
    if (fwrite(db, 1, sizeof(ImageDatabase), db->file) != 1) {
        fprintf(stderr, "unable to write newly created database to %s", filename);
        goto write_fail;
    }
    return db;

//error handling (the order of the label is reversed, it follow the order of pop on a stack)
write_fail:
     free(db->metadatas); //pop resource3
metadata_fail:
     fclose(db->file); //pop resource2
file_fail:
     free(db); //pop resource1
db_fail:
     return NULL;
}

###Snippet 4 This version (22 lines of code) uses goto to achieve error handling without duplication and without deep nesting and two macros to shorten the code and to avoid mixing the logic with the error handling. The code has the following structure:

try( allocate resource and check for error, label_for_error); // push on the (imaginary) stack
<further code using the resource>

LABEL(label_for_error, code_to_execute_once_if_try_failed); // pop the (imaginary) stack
  • Upside: the shortest version and doesn't mix error signaling/handling with logic
  • Downside: uses two (short) macros

The macros:

#define try( stmt, label )           \
    do{                              \
        if(!(stmt)){ goto label; } } \
    while(false)                    
    
#define LABEL( label, execute_once_code )        \
    do{                                          \
        if(false){ label: execute_once_code; } } \
    while(false)

The code:

ImageDatabase*
db_create_try(char const* filename,
           uint32_t max_images,
           struct resolution resolutions[NB_RESOLUTIONS - 1])
{
    //logic is not mixed with error handling:
    
    ImageDatabase* db;
    try( db = allocate_empty_db(), db_fail);
    memcpy(db->resolutions, resolutions, (NB_RESOLUTIONS - 1) * sizeof(struct resolution));
    db->max_nb_images = max_images;
    try( db->file = fopen(filename, "r+b"), file_fail);
    try( db->metadatas = calloc(max_images, sizeof(struct metadata)), metadata_fail);
    if (fwrite(db, 1, sizeof(ImageDatabase), db->file) != 1) { goto fwrite_fail; }
    return db;
    
    //error handling:       (order is the same as in snippet 3)
    
    LABEL( fwrite_fail, fprintf(stderr, "unable to write newly created database to %s", filename));
    free(db->metadatas);
    
    LABEL( metadata_fail, fprintf(stderr, "unable to allocate metadatas"));
    fclose(db->file);
    
    LABEL( file_fail, fprintf(stderr, "unable to open %s", filename));
    free(db);
    
    LABEL( db_fail, fprintf(stderr, "unable to allocate database"));
    return NULL;
}

How about using a library ?

see here : http://www.throwtheswitch.org/cexception

@julien-h
Copy link
Author

julien-h commented Dec 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment