Difference between revisions of "MansOS Coding Standard"
(→Inline comments) |
(→Comments in global scope) |
||
Line 21: | Line 21: | ||
uint32_t gcd(uint32_t a, uint32_t b); |
uint32_t gcd(uint32_t a, uint32_t b); |
||
In general, there is |
In general, there is no need to document function definitions. All exported functions should have prototypes. |
||
==== Preprocessor directives and comments ==== |
==== Preprocessor directives and comments ==== |
Revision as of 23:45, 14 January 2010
General issues
Comments
Respect the reader and write comments. In general, comment what your code does, not how.
Inline comments
Inside functions use comments only when necessary. In most cases, if you see some code that cannot be understood without commenting, that means you should rewrite it, not comment it.
Use descriptive function and variable names to reduce the need to comment your code.
Comments are not checked by compiler. That means they are often "broken" - invalid or out of date. And the more comments there are, the more difficult it is to keep them up to date.
Comments in global scope
Describe what a function does before to it's prototype, unless that's obvious.
Example (quite obvious):
// calculate greatest common divisor (GCD) of 'a' and 'b' uint32_t gcd(uint32_t a, uint32_t b);
In general, there is no need to document function definitions. All exported functions should have prototypes.
Preprocessor directives and comments
When using #ifdef
or #if
constructs, you should always use comments for #endif
.
The comment for #endif
should match the expression used in the corresponding #if
or #ifdef
. The comment for #else
and #elif
should match the inverse of the expression(s) used in the preceding #if
and/or #elif
statements.
#ifdef MSP430
/* msp430 stuff here */
#else // !MSP430
/* non msp430 stuff here */
#endif // MSP430
Special comments
Use:
- XXX in comments to mark code that may not be strictly correct, but is working.
- FIXME in comments to mark code that does not work correctly.
- TODO in comments to mark places for not-yet-made features.
Indentation
Do not use the TAB character for indentation, use spaces instead. Otherwise the code indentation may break when viewed by different developers.
Use indentation at 4 spaces. You may find it useful to set TAB key to 4 spaces in your editor.
Cross platform code
Try to put all platform specific code in appropriate .h or .c files; avoid having #ifdef
's in code, when possible.
Use:
... pc/platfor_header.h: #define PLATFORM_SPECIFIC_CONST 1 ... telosb/platfor_header.h: #define PLATFORM_SPECIFIC_CONST 2 ... .c file: a = PLATFORM_SPECIFIC_CONST;
Avoid:
... .c file #ifdef PC a = 1; #else a = 2; #endif
Inline functions
Linux coding style manual suggests that "a reasonable rule of thumb is to not put inline at functions that have more than 3 lines of code in them". For embedded systems this suggestion may be relaxed (to avoid function call overhead), but they should be kept at reasonable size.
The compiler can make decision to inline even functions that are not declared as inlines, but only if the definition of that function is visible at the point where it is called.
Warnings
Do not ignore compilation warnings. Never commit code that compiles with warnings, unless you have a very good reason!
If there are really "invalid" warnings, it's better to disable them in Makefile than to ignore them.
Types
Constants and defines
Define constants with #define preprocessor directive or (preferred) as enum member.
The enum way is preferred because a symbol table entry is created in this way (can be useful for debugging), and the scope of the declaration is honoured.
Do no use const keyword, except in cases where it is really needed. const keyword should not be used, because in this way not only a symbol, but also a variable is always created. Note that this only applies to C, for C++ const can be used to make real constants.
Use all-uppercase names for constants, with underscore between the words.
Use:
#define DEFINED_C 123u // acceptable enum { ENUM_C = 123 }; // preferred
Avoid:
const unsigned VARIABLE_C = 123;
Avoid magic numbers. Use symbolic constants, defined in one way or another.
Integer types
Use only in integer types with explicitly specified size. This makes portability easier.
Use:
int8_t a; uint32_t b;
Avoid:
char a; unsigned long b;
Bool type
Use bool
type where possible.
Error code type
[TODO]
Syntax
Variable and function names
Variable names and function names shall start in lowercase and be written in CamelCase - writeByte
, not write_byte, wrt_byte
or WrItEbYtE
.
int writeByte;
void printByte(void *buf);
Enums, structs and all other types
Use the first letter in uppercase, the rest is CamelCase. Add trailing "_t
" to show this is a type, or "_e
" to show this is an enum.
typedef struct
{
int a;
int b;
int c;
} MyStruct_t;
typedef enum
{
RECTANGLE,
CIRCLE = 17,
LAST // Use "Last" as the last enum available, as needed
} ShapeType_e;
Enumeration values all should be written in uppercase.
Code control structures
Using braces
Always use braces after if(), else, for(), while(), do
, even when there is only one statement in the block. There might be a few exceptions when if and the statement is on the same line and unmistakeably has one simple statement such as an assignment.
You may write the opening brace on the same or the next line. Use common sense. Generally to make a short if()
with just one statement in the body I use it on the same line.
Use:
if ( 17 == a ) b++; if (...) { foo(); } if (...) { foo(); bar(); }
Avoid:
if (foo) bar();
This should be avoid because the code is harder to modify (e.g. add more statements insides the body of the if
statement) in the future; and because the code flow may be unclear in some cases.
switch
statement
In all places where break
keyword is not being used by intention, a /* falls through */
comment should be inserted.
Example:
switch (condition) { case A: if (x == 13) return 5; // falls through case B: case C: return 6; }
Spacing in expressions
Use space to separate tokens in expressions and braces everywhere in the code. However, you may use no space between unary operation and operand, for example, i++;
x = y + 16 - z++;
if ( 17 == a ) { b = foo( c, d ); }
Use spaces before braces with if, for, while
.
Do not use space before braces in foo()
(i.e. function calls).
Use spaces around binary and ternary operators:
x = a ? b + c : d;
Size limits
Keep the line length under 80 characters to help the viewer and those that want to print your code. Break the line in several as necessary.
Functions in general should be no longer than ~50 lines (one screen). Functions that has only simple logic (e.g. initialization function) can be longer.
Try to not use too much levels of indentation - code is harder to read if there are too many.
Mixing variables and code
Use C89 style; declare variables before code.
Use:
uint32_t foo;
do_something();
foo = bar();
Or:
uint32_t foo = bar();
do_something();
Avoid:
do_something();
uint32_t foo;
foo = bar();
Variable initialization
No explicit global variable initialization to zeros need, because by C standard all static variables (including global) are by default initialized to zero.
If for some reason you want to avoid initalization, use "noinit" attribute. For example, this may be need for large arrays, because apparently the initialization can take long enough time for watchdog timer to expire!
Example:
uint16_t array[500] __attribute__((section (".noinit")));
For some compilers there is __no_init keyword that does the same:
__no_init uint16_t array[500];
Structures can be initialized like this:
typedef struct { uint16_t i; int8_t c; uint8_t a[3]; } SomeStruct_t; SomeStruct_t var1 = {123, 'a', {1, 2, 3}}; SomeStruct_t var2 = {123}; // member 'i' is 123, all other members are implicitly initialized to 0
Or (the preferred way):
SomeStruct_t var3 = { .i = 123, .c = 'a', .a = {1, 2, 3} };
Include paths
All internal MansOS code should use include paths with directory explicitly specified. E.g. use
#include <lib/dprint.h>
Avoid:
#include <dprint.h> #include "dprint.h"
This is not a requirement for application code.
This is also not a requirement if the included file is under HAL or HPL directories, because in this case the path is platform specific. Because of this, it is recommended to add "_hal"
or "_hpl"
suffix in those filenames, for example, xxx_hal.h
and yyy_hpl.h
.
Conclusion
Remember that rules are made to be broken; do not follow them blindly!
Sample Emacs configuration
MansOS C mode for Emacs (add this to your Emacs configuration file):
(defun mansos-c-mode () "C mode for MansOS" (interactive) ; use C++ mode, for "//" comments (c++-mode) ; convert tabe to spaces when typing (setq indent-tabs-mode nil) (setq default-tab-width 4)) ; Change the path to your MansOS repository! (setq auto-mode-alist (cons '("/home/atis/sensors/mansos/.*\\.[ch]$" . mansos-c-mode) auto-mode-alist))