nasa / bplib

Apache License 2.0
27 stars 13 forks source link

Avoid inline "return" statements after initial error checking #40

Open jphickey opened 4 years ago

jphickey commented 4 years ago

In many functions if an intermediate step fails, it will invoke a "return" statement in the middle of the function.

This can be problematic particularly in cases where a clean-up needs to be done. In particular, if a lock is taken or a resource is held, this resource needs to be released before the returning.

Although this can be considered a style issue, it is really for better maintainability of the code and reducing the chance of missing the important clean-up steps.

Some developers use goto statements for this, but another common trick is to use a do {} while(0); construct with break statements, such as:

if (/* some basic parameter test */)
{
   return NULL;
}

do
{
    /* first resource allocation */
   if (/* failed */)
   {
       break;
    }

    /* second resource allocation */
   if (/* failed */)
   {
       break;
    }

   /* remaining allocations... */

}
while (0);

if (/* anything failed */)
{
    /* do complete cleanup of everything that was allocated */
   return NULL;
} 

The key is to have only one implementation of the clean-up code, which does everything, and does it reliably and can be verified and confirmed as such. The problem of inline returns is that the clean up code tends to be copied many times over, and also tends to get "bigger" after each allocation. All these separate implementations need to be verified somehow, too.