Coding Style Guidelines

This document defines the structure of the NxOS source code, both in its general organization and internal formatting details.

General structure

The code is structured into a baseplate and a set of application kernels. Specifically:

  • base contains the baseplate kernel (all things that are not tied to hardware)
  • base/drivers contains the baseplate device drivers
  • nxos/systems contains all the application kernels, one subdirectory each.

Source files should be concisely named based on their function (eg. interrupts.S for interrupt handling).

If the source file exports functions for reuse, create and populate a corresponding .h file (eg. interrupts.h)

If the source file needs to share internal definitions with other parts of the system, create and populate a .h file whose name is the name of the public include file prefixed with an underscore (eg. _interrupts.h).

Headers

Header files should all contain the same boilerplate: 

/** @file filename.h
 *  @brief What does this component do?
 *
 * Optionally, more stuff about the component in general.
 */

/* Copyright (C) 2007 the NxOS developers
 *
 * See AUTHORS for a full list of the developers.
 *
 * Redistribution of this file is permitted under
 * the terms of the GNU Public License (GPL) version 2.
 */

#ifndef __COMPONENT_FILENAME__
#define __COMPONENT_FILENAME__

/* Declarations go here. */

#endif /* __COMPONENT_FILENAME__ */

A few examples of the preprocessor symbol for various files:

All typedefs, enumerations, structures, macros and preprocessor defines should be documented, using doxygen's "javadoc" style syntax (/** */, @foo...).

It is strongly encouraged to define Doxygen groups, to group functions into logical components, with a documentation string for the whole group. Since our code is C, these documentation structures are in a way "classes" to our users, that help them construct a hierarchical view of our API.

C Sources

C sources should follow the following pattern. This is a little less rigid than the header files, since cases may vary. The order of includes should, however, be respected: first the hardware include, then base/ includes, then base/drivers, and last the include that matches the source file. 

/* Copyright (C) 2007 the NxOS developers
 *
 * See AUTHORS for a full list of the developers.
 *
 * Redistribution of this file is permitted under
 * the terms of the GNU Public License (GPL) version 2.
 */

/* If hardware registers are accessed */
#include "base/at91sam7s256.h"

/* Include as necessary */
#include "base/foo.h"
#include "base/bar.h"
#include "base/drivers/somedevice.h"

/* Include last, with the correct path. */
#include "path/to/header_for_this_file.h"

/* Internal enums, defines and structures go here */

/* Internal functions go here, defined static */

/* Functions declared internal (in _header.h) go here */

/* Public API functions go here */

C coding guidelines

Indentation

Use 2 spaces for each level of indentation. Never use tabs.

Some editors will let you insert 2 spaces, but will convert 8 spaces to a tab. This breaks source code formatting for people who have a different tab width configured. Please make sure that you disable all tab insertions in your editor.

Includes

Inclusions should be from the root of the build tree for Baseplate code. 

/* Good, fully qualified include */
#include "base/drivers/motors.h

/* Bad, unqualified include */
#include "motors.h"

/* Bad, partially qualified include */
#include "drivers/motors.h"

Application kernels should only include public headers (no leading underscore).

Documentation

Do not use Doxygen markers in .c files. Comment with "normal" comments as you see fit. NxOS does not subscribe to "it was hard to write, so it should be hard to read".

Function and variable names

Use lowercase and underscores only in variable names. Do not use mixed case or upper case notations. 

/* Good */
U32 my_variable;
void my_function(void);

/* Bad */
U32 myVariable;
U8 MyVariable;
void my_Function(void);

Functions that are defined static (scoped to a single file) do not need to use any prefixes such as nx_. Name them concisely for brevity, since there is no risk of symbol conflicts.

Integer types

If no external constraints apply, prefer the U32 or S32 data types for integers over their 8 and 16 bit equivalents. The reason is that registers on the ARM architecture are 32 bits wide, so using smaller data types forces the compiler to add more code to mask the out-of-bound bits if you don't use 32 bit data types.

Pointer types

The pointer star should be stuck against the variable name. 

/* Good */
U32 *foo;

/* Bad */
U32* foo;
U32 * foo;

Macros

Avoid using macros whenever possible. Instead of defining a macro, it is usually preferable to define a static inline function. Instead of defining a preprocessor symbol, it is usually preferable to define a static constant in the file scope. If you really must define a macro or preprocessor symbol, it should use uppercase letters. 

/* Good */
static inline clock_speed(U32 mhz) {
  return mhz * 42 / 16;
}
static const U32 foo = 42;

/* Bad */
#define CLOCK_SPEED(mhz) (mhz * 42 / 16)
#define FOO 42

/* Even worse. It will not be evident in the code that foo(1) is a macro. */
#define foo(evil) (evil + evil)

In some cases the use of a macro may be necessary to simplify syntax, but please only use them only if you are sure you have no other choice (feel free to ask for advice from other developers!).

Functions

Do not put a space before the opening parenthesis of a function definition or function call. void foo(void) is good, not void foo (void). 

/* Good */
void foo(void);

/* Bad */
void foo (void);

Braces

The opening braces should not be on a line by themselves. Closing braces should. 

/* Good */
void foo(U32 bar) {
...
  if (condition) {
  ...
  }
}

/* Bad */
void foo(U32 bar)
{
...
  if(condition)
  {
    ...;
}}

Accessing registers

When accessing registers defined in base/at91sam7s256.h, use the direct register name, not the per-peripheral struct. 

/* Good */
AT91C_PMC_PCDR = 0;

/* Bad */
AT91C_BASE_PMC->PMC_PCDR = 0;

Device driver specific guidelines

State structure

Most device drivers have a static struct containing all the driver's state. Use such a struct instead of defining free static variables. 

/* Good */
static struct {
  U32 foo;
  U8 *bar;
} mydriver_state;

/* Bad, don't let driver state float freely in the file. */
U32 foo;
U8 *bar;

The state structure should be named drivername_state. For example, the state struct for the AVR driver is called avr_state.