Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why is this construct used? Mad or genius?

Tags:

c

coding-style

I'm working with a large SDK codebase glommed together from various sources of varying quality / competence / sanity from Linus Torvalds to unidentified Elbonian code slaves.

There are an assortment of styles of code, some clearly better than others, and it's proving an interesting opportunity to expand my knowledge / despair for the future of humanity in alternate measures.

I've just come across a pile of functions which repeatedly use a slightly odd (to me) style, namely:

void do_thing(foo)
{
    do {
        if(this_works(foo) != success)
            break;
        return(yeah_cool);
    } while (0);
    return(failure_shame_death);
}

There's nothing complicated being done in this code (I haven't cut 10,000 lines of wizardry out for this post), they could just as easily do:

if(this_works(foo) == success)
    return(yeah_cool);
else
    return(failure_shame_death);

Which would seem somehow nicer / neater / more intuitive / easier to read.

So I'm now wondering if there is some (good) reason for doing it the other way, or is it just the way they always do it in the Elbonian Code Mines?

Edit: As per the "possible duplicate" links, this code is not pre-processed in any sort of macro, it is just in the normal code. I can believe it might be due to a coding style rule about error checking, as per this answer.

like image 447
John U Avatar asked Oct 17 '13 10:10

John U


4 Answers

Another guess: maybe you didn't quote the original code correctly? I have seen the same pattern used by people who want to avoid goto: they use a do-while(0) loop which at the end returns a success value. They can also break out of the loop for the error handling:

int doXandY() {
   do {
      if (!x()) {
         break;
     }

     if (!y()) {
         break;
     }
     return 0;
   } while( 0 );

   /* Error handling code goes here. */
   globalErrorFlag = 12345;
   return -1;
}

In your example there's not much point to it because the loop is very short (i.e. just one error case) and the error handling code is just a return, but I suspect that in the real code it can be more complex.

like image 137
Frerich Raabe Avatar answered Nov 09 '22 08:11

Frerich Raabe


Some people use the do{} while(0); construct with break; inside the loop to be compliant in some way with MISRA rule 14.7. This rule says that there can be only single enter and exit point in the function. This rule is also required by safety norm ISO26262. Please find an example function:

int32_t MODULE_some_function(bool first_condition,bool second_condition)
{
    int32_t ret = -1 ;

    do
    {
        if(first_condition)
        {
            ret = 0 ;
            break ;
        }

        /* some code here */ 

        if(second_condition)
        {
            ret = 0 ;
            break ;
        }

        /* some code here */ 

    } while(0) ;

    return ret ;
}

Please note however that such a construct as I show above violates different MISRA rule which is rule 14.6. Writing such a code you are going to be compliant with one MISRA rule, and as far as I know people use such a construct as workaround against using multiple returns from function.

In my opinion practical usage of the do{}while(0); construct truely exist in the way you should construct some types of macros.Please check below question, it was very helpful for me :

Why use apparently meaningless do-while and if-else statements in macros?

It's worth notice also that in some cases do{}while(0); construct is going to be completely optimized away if you compile your code with proper optimization option.

like image 41
Lazureus Avatar answered Nov 09 '22 07:11

Lazureus


Hm, the code might be preprocessed somehow. The do { } while(0) is a trick used in preprocessor macros; you can define them like this:

#define some_macro(a) do { whatever(); } while(0)

The advantage being that you can use them anywhere, because it is allowed to put a semicolon after the while(0), like in your code above.

The reason for this is that if you write

#define some_macro(a) { whatever(); }

if (some_condition)
    some_macro(123);
else
    printf("this can cause problems\n");

Since there is an extra semicolon before the else statement, this code is invalid. The do { ... } while(0) will work anywhere.

like image 5
Johan Henriksson Avatar answered Nov 09 '22 06:11

Johan Henriksson


do {...} while(0) arranged with "break" is some kind of "RAII for Plain C".

Here, "break" is treated as abnormal scope exit (kind of "Plain C exceptions"), so you can be sure that there is only one place to deallocate a resource: after a "while(0)". It seems slightly unusual, but actually it's very common idiom in the world of plain C.

like image 1
Yury Schkatula Avatar answered Nov 09 '22 08:11

Yury Schkatula