4
\$\begingroup\$

I'm starting a new C projet and I want to have the cleanest and shortest error checking code possible.

To that aim, I often use two things :

  1. functions return 0, false or NULL on error
  2. I try to use small functions with one cleanup label "fail" or "error"

So I thought about using a macro to simplify the code even more. If I choose to use it, this pattern will appear in almost every function so I expect that people reading the code will become accustomed to it very quickly.

So, is it a viable solution ? Here is an example snippet :

#define try( stmt , error_label ) if (!(stmt)) { goto error_label; }

// this function creates a new image database
ImageDatabase*
db_create(uint32_t max_images,
          struct resolution resolutions[NB_RESOLUTIONS])
{
    ImageDatabase* db;
    try( db = allocate_empty_db(), error);
    memcpy(db->resolutions, resolutions, NB_RESOLUTIONS * sizeof(struct resolution));
    db->max_nb_images = max_images;
    try( db->metadatas = calloc(max_images, sizeof(struct metadata)) , error);
    try( finalize_creation(db) , error);
    return db;

error:
    free(db->metadatas);
    free(db);
    return NULL;
}

EDIT: Here is an alternative without the try macro... but it's not as pretty. Also note that in this example the code is simple enough to avoid a goto, but it's not always the case.

ImageDatabase*
db_create(uint32_t max_images,
          struct resolution resolutions[NB_RESOLUTIONS])
{
    ImageDatabase* db = allocate_empty_db();
    if (db) {
        memcpy(db->resolutions, resolutions, NB_RESOLUTIONS * sizeof(struct resolution));
        db->max_nb_images = max_images;
        db->metadatas = calloc(max_images, sizeof(struct metadata)) , error);
        if (db->metadata == NULL
           || !finalize_creation(db)) 
        {
            free(db->metadatas);
            free(db);
            db = NULL;
        }
    }
    return db;
}
\$\endgroup\$
2
\$\begingroup\$

NULL dereference

If allocate_empty_db() returns NULL, it will jump to the error handling code, which will cause a NULL dereference when it tries to free(db->metadatas). You will need to guard that line like this:

if (db)
    free(db->metadatas);

The macro

I'm not exactly a fan of the macro. First, calling it try makes me think your code is java, and then when I realize it isn't, I'm more confused. Second, if your label is always going to be the same in every function, then you can omit the second argument of the macro and just hardcode it to goto error. That way the macro can look smaller and the reader doesn't have to parse the extra comma and argument (which look like part of the body you are try-ing).

Also, the macro should be a do while instead of an if. You could get in trouble with code like this:

if (condition)
    try(foo(), error);
else
    bar();

Here, your macro will cause the above to expand to:

if (condition)
    if (!foo()) { goto error; };
else
    bar();

Which is actually a syntax error because of the extra semicolon but is also close to making the else pair with the wrong if. The macro should look like this instead:

#define try( stmt , error_label )              \
        do {                                   \
            if (!(stmt)) { goto error_label; } \
        } while(0)

This do while(0) construction is a common trick when writing C macros. It allows a multi-statement macro to be used in an if statement like the one in my example. See this Stackoverflow question for more info.

\$\endgroup\$
0
\$\begingroup\$

Your code using a macro just doesn't look like C, whereas the code it replaces does. And while you think your code looks prettier, many will disagree. Error handling in C is difficult and messy. But people expect to see it spelled out not hidden away.

So I'd say if you are writing this project with other people, don't use this macro. It will wind-up many programmers (as do personalized indenting styles and other non-standard approaches) and I don't think it will really make life easier.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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