A critique of the Barr Group Embedded C Coding Standards

Here, I present my personal thoughts on what I view as many shortcomings of the popular Barr Group Embedded C Coding Standards. What follows is non-exhaustive list and rebuttal of many of the points with which I personally disagree.

1.1.a. Use the latest version of C supported by your compiler. C18 has some great features that aren't in C99. Don't restrict yourself to supporting out-of-date compilers if you don't have to.

1.1.b. Don't use a C++ compiler to compile C. Use a C++ compiler to compile C++, and a C compiler to compile C.

1.3.b. Personal preference. I prefer my braces to hug their conditions, as it makes it more obvious if there is a stray semicolon before the curly brace, and clearly indicates that an else or while (of a do/while) pairs with the preceding block.

1.6.a. Casts should be avoided whenever possible. The example provided doesn't even make sense, since sample is declared as uint16_t, there is no reason to call abs() on it.

1.7.d. The continue keyword has several practical use-cases: explicitly describing an empty loop and early termination of a loop are just two.

2.1.b. It's perfectly acceptable for a comment to include //. It's not going to become any less of a comment that way. The \ is only an issue if it's the last character on the line. But try to describe a Windows path without one. Embedding /* in a comment is problematic, though. Barr Group is awarded 1/3 credit on this one.

2.1.c. Code should never be checked in to source control commented out. But for the edit-compile-test cycle, this is a totally valid strategy in tracking down some bugs.

3.1.e. Never put a space after the * in a pointer declaration.

3.1.j. Never put a space between function names and the parenthesis, even in declaration. Learn to use ctags.

3.2.a. LOL, no. Just put one space between the type and the variable name.

3.2.b. Same.

3.2.c. This contradicts the otherwise sound advice regarding whitespace surrounding operators from the previous section.

3.2.d. If it makes sense to indent a preprocessor directive, it makes sense to indent the entire preprocessor directive.

3.3.c. No. Let your editor tell you where the end of the file is (hint: G).

3.4.a. Each indentation level is one tab. Tabs are 8 spaces.

3.4.b. The case labels should be indented at the same level as the switch statement.

3.5.a. Indentations are done with tabs, explicitly stored as a tab character. Fight me.

3.6.b. Form feed serves no purpose in source code. It shouldn't be there.

4.1.d. It's perfectly reasonable for the file that contains main() to be named your-program-name.c.

4.2.a. Each header file should describe a complete interface. Feel free to decide how the implementation should be divided.

4.2.b. Don't even think of naming your #include guards with leading underscores.

4.3.b. Structure your code in a way that makes sense to you. I prefer static functions defined first, without separate prototypes, so there is one less chance for things to get out of sync if the interface needs to change.

4.3.c. The header file need not be of the same name, if the implementation is broken into several different translation units.

4.3.f. Sometimes it makes sense to #include another source file. Unity builds come to mind. Poor-man's templating is another use case.

4.4.a. Any project that requires a template for source code is too complex.

5.1.a. POSIX reserves type names ending with _t. That means you don't get to use them. I have actually seen this be a problem in real world code.

5.1.b. Don't typedef structs or unions, only scalar types. It forces the consumer to think about the fact that they are using aggregate types whenever they are declared. The exception is for pointers to opaque types (like FILE*).

5.2.b. Sometimes you have to use short or long to conform to an API. But, they should be used sparingly.

5.2.c. Using char is also a great way to declare a buffer that is just going to be used as a bag of bits (though unsigned char might be more appropriate).

5.4.b.i. This is the most egregiously wrong guideline in the entire bunch. No standard defines types named float32_t, float64_t, or float128_t. Not C99, not C11, not C18, not even the current proposal for C20. Just use float, double, and long double.

6.1.a. It is, in fact, a good idea to write syntactically correct programs. They stand a much better chance of compiling that way.

6.1.e. The choice of camel-case is a matter of personal preference.

6.1.f. Function-like macros can and should have lowercase names. There's no reason to scream out that they are macros if they behave appropriately (e.g. don't evaluate parameters multiple times). Macros used as a constants should be all upper case, though.

6.2.a. Functions should be no longer than they need to be, but there is no fixed maximum. Use judgement. If the function is doing multiple things, break it into separate functions, one per actual step of the process.

6.2.b. Nobody cares how your code looks on paper. Unless it's a PostScript program, I guess.

6.2.c. Functions that require some sort of clean-up (e.g. freeing memory, closing files) should have a single point of clean-up. Otherwise, early return is a fantastic tool.

7.1.e. The loop index names are i and j (and, in extreme cases, k). Anything more is not idiomatic, and unnecessarily verbose with no added benefit.

7.1.g. Avoid global variables. But, if they are necessary, give them long, descriptive names. There is no need to prefix them.

7.1.k. Do not use anything resembling Hungarian notation.

7.1.l. Do not use anything resembling Hungarian notation.

7.1.m. Do not use anything resembling Hungarian notation.

7.1.n. Do not use anything resembling Hungarian notation.

7.1.o. The order of prefixes doesn't matter when you Do not use anything resembling Hungarian notation.

8.2.c. Sometimes it makes sense to do assignments in an if clause, though your compiler should warn you if you do this without also checking the value of that assignment against some constant.

8.2.d. An empty else clause "for completeness" is pointless. If you can prove that all possible interesting cases are handled by the if and else if clauses, leave out the else.

8.3.a. The break statement aligns with the code it follows, and should be one level deeper than the case label.

8.3.b. There need not be a default label if all interesting cases are provably handled.

8.4.a. Zero is not a magic number.

8.4.b. It's fine to make assignments in a test clause, just do it correctly.

8.5.b. The only part of the standard library that is off-limits is gets(), which is so prone to error that it was actually removed in C11, the only part of the library to be so dealt with.

8.6.a. Put the thing that you are interested in testing on the left side of the comparison. Nobody is ever interested in the value of NULL. Your compiler should warn you if you mistakenly place an assignment in a condition (and if it doesn't, turn on warnings, now).

Copyright © 2020 Jakob Kaivo <jakob@kaivo.net>