Site logo
Log inPremiumDiscover Hola Products

Support

C Coding Convention

Improvements? Suggestions? email dna@hola.org

Consistent and Minimal

Most important rules: Be consistent. Be minimal. Read these sections carefully.

Text file layout

Tab size is 8 spaces.
Shift width (indentation) is 4 spaces.
Column width 79 char, so it fits well with 80 col terminal.
Indentation is always one shift-width(4):

printf("closing file %s on server\n",
        file_name);

printf("closing file %s on server\n",
    file_name);

Braces

open and close braces of a section should be on the same level.

if (close_files) {
    fclose(fp1);
    fclose(fp2);
}

if (close_files)
{
    fclose(fp1);
    fclose(fp2);
}

Breaking a long line

when breaking up a long line, it should continue with one shift-width for indentation

if (line_length>1 && (screen->sz->vertical<buffer->sz->vertical
                      || explicit_lines))
{
    printf("this is a test section that will show how to handle "
           "long lines, such as %s which is %d lines long",
           "this", 2);
    SetWindowText( hWnd,
                   "Button Name" );
}

if (line_length>1 && (screen->sz->vertical<buffer->sz->vertical
    || explicit_lines))
{
    printf("this is a test section that will show how to handle "
        "long lines, such as %s which is %d lines long", "this", 2);
    SetWindowText(hWnd, "Button Name");
}

if for while statement

if/for/while statement that takes more than one line should always have braces; same thing goes when the then/loop part is more than one line

for (ret=0; ret<pciScan.dwCards;ret++)
    if (slot.dwBus==pciScan.cardSlot[ret].dwBus &&
        slot.dwSlot==pciScan.cardSlot[ret].dwSlot &&
        slot.dwFunction==pciScan.cardSlot[ret].dwFunction)
        break;

if (slot.dwBus==pciScan.cardSlot[ret].dwBus &&
    slot.dwSlot==pciScan.cardSlot[ret].dwSlot &&
    slot.dwFunction==pciScan.cardSlot[ret].dwFunction)
{
    break;
}

if (x==y)
{
    my_func(param1, param2, param3, param4, param5, param6,
        param7);
}

Functions prototypes space

function prototypes should be spaced like function calls

void do_keyboard_input( int key,
                        char *name,
                        int list_box,
                        BOOL is_read,
                        BOOL is_password )
{
    // functions code
}

void do_keyboard_input(int key, char *name, int list_box, BOOL
    is_read, BOOL is_password);

// in header file
void do_keyboard_input(int key, char *name, int list_box,
    BOOL is_read, BOOL is_password);

// in code file
void do_keyboard_input(int key, char *name, int list_box,
    BOOL is_read, BOOL is_password)
{
    // functions code
}

// if you need to comment parameters
void do_keyboard_input(
    int key, // Encryption key
    char *name, // encrypted user name
    int list_box, // list of destinations to choose from
    BOOL is_read,
    BOOL is_password) // destination specific psw
{
    // functions code
}

Unneeded casting

do not cast void *, this is unneeded in C.

estream = (estream_t *)malloc(sizeof(*estream));

estream = malloc(sizeof(*estream));

Do not cast function prototypes

do not cast function prototypes - if the implementation changes in the future, it will be hard to discover. the only exception is typecasting xxx_free() functions (functions from type void xxx_close(xxx_t *xxx)). 

typedef void (*etimer_func_t)(void *);
void event_timer_set(double ms, etimer_func_t func, void *data);

void estream_read(estream_t *estream)
{
    read(estream->fd);
}

void do_something(void)
{
    estream_t *estream;
    event_timer_set(0, (etimer_func_t) estream_read, estream);
}

void estream_read(estream_t *estream)
{
    read(estream->fd);
}

void estream_read_et(void *o)
{
    estream_read((estream_t *)o);
}

void do_something(void)
{
    estream_t *estream;
    event_timer_set(0, estream_read_et, estream);
}

void estream_read(void *o)
{
    estream_t *estream = o;
    read(estream->fd);
}

void do_something(void)
{
    estream_t *estream;
    event_timer_set(0, estream_read, estream);
}

switch statement

switch statements should have the case on the same level, no space before ':'.
if the case is one line, then one space after ':'.
never 'break' in 'default'.

switch (selection)
{
    case KEY_UP :
        x--;
        break;
    case KEY_DOWN :
        x++;
        break;
}

switch (selection)
{
case KEY_UP:
    x--;
    break;
case KEY_DOWN:
    x++;
    break;
}

switch (selection)
{
case KEY_UP: x--; break;
case KEY_DOWN: x++; break;
}

switch (selection)
{
case KEY_UP: x--; break;
case KEY_DOWN: x++; break;
default: invalid = 1; break;
}

switch (selection)
{
case KEY_UP: x--; break;
case KEY_DOWN: x++; break;
default: invalid = 1;
}

switch (selection)
{
case KEY_UP: key = UP; break;
case KEY_DOWN: key = DN; break;
case KEY_LEFT: key = LEFT; break;
case KEY_RIGHT: key = RIGHT; break;
default: key = NONE;
}

Spaces

no space between function name and opening parenthesis
no space between opening parenthesis and first parameter
one space after comma: 

printf ("hello %s\n", "world");
printf( "hello world\n" );
printf("hello world\n","world");

printf("hello %s\n", "world");

one space after reserved words before the opening parenthesis:

if(close_file)

if (close_file)

'then' part of if statement

'then' part of if statement should be in a separete line:
reason: allows to debug the if and see when it is run.

if (close_file) fclose(fp);

if (close_file)
    fclose(fp);

else if

else if statements should be on the same level at the starting if reason: this is similar to switch statement

if (!strcmp(argv[1],"--help")) print_usage();
else if (!strcmp(argv[1], "--run")) {
    run_application();
    print_results();
} else print_error();

if (!strcmp(argv[1], "--help"))
    print_usage();
else if (!strcmp(argv[1], "--run"))
{
    run_application();
    print_results();
}
else
    print_error();

return

return statements should not have parentheses and should not have a space after them:

return (0);

return 0;

return ;

return;

do not use unneeded parentheses in expressions

if ((a!=b) || (c!=d))
    ...

if (a!=b || c!=d)
    ...

do not call return at the end of a function returning void

void f(void)
{
    printf("hello");
    return;
}

void f(void)
{
    printf("hello");
}

should not separate with more than one blank line between sections, functions etc.

structure and enum definitions

structure and enum definitions, itializations should open { at the same line. it is recommended that the name will end with a _t. typedef the struct, to make it easy to use later on. 

typedef struct _WD_INTERNAL    {
    int setup;
    int offset;
} WD_INTERNAL;
typedef struct
{
    int setup;
    int offset;
} wd_internal_t;
struct wd_internal_t {
    int setup;
    int offset;
};

// when forward structure definition needed:
typedef struct wd_internal_t {
    struct wd_internal_t *next;
    int setup;
    int offset;
} wd_internal_t;

typedef struct {
    int setup;
    int offset;
} wd_internal_t;

wd_internal_t wd_initial = {
    8,
    16,
};

Comments

prefer C++ comments to C comments

/* close all files */
fclose(fp);

// close all files
fclose(fp);

Comments aligned

comments should be aligned as the code they comment, or one space after the end of the line.

/*
 * close all files
 */
fclose(fp);

//
// close all files
//
fclose(fp);

    /* close all files */
    fclose(fp);
GOOD better:
    // close all files
    fclose(fp);

fclose(fp); // close all files

Multiline comments

comments which occupy more than one line should adhere to the following guidline

//
// close all the files that were opened before the function was called
// and send them to the output file
//
fclose(fp);

// close all the files that were opened before the function was called
// and send them to the output file
fclose(fp);

Comments ident

Comment should never be ident more the 4 spaces even in structures.
If longer, put the comment at the line above.

typedef struct {
    isa_pnp_id search_id; /* if search_id.vendor[0]==0 - scan all vendor
                           * IDs
                           * if searchId.dwSerial==0 - scan all serial
                           * numbers */
    int cards; /* number of cards found */
    isa_pnp_card card[ISAPNP_CARDS]; /* cards found */
} wd_isapnp_scan_cards_t;

typedef struct {
    isa_pnp_id search_id;            /* if search_id.vendor[0]==0 - scan
                                      * all vendor IDs
                                      * if searchId.dwSerial==0 - scan all
                                      * serial numbers */
    int cards;                       /* number of cards found */
    isa_pnp_card card[ISAPNP_CARDS]; /* cards found */
} wd_isapnp_scan_cards_t;

typedef struct {
    /* if search_id.vendor[0]==0 - scan all vendor IDs
     * if searchId.dwSerial==0 - scan all serial numbers */
    isa_pnp_id search_id;
    int cards; /* number of cards found */
    isa_pnp_card card[ISAPNP_CARDS]; /* cards found */
} wd_isapnp_scan_cards_t;

typedef struct {
    // if search_id.vendor[0]==0 - scan all vendor IDs
    // if searchId.dwSerial==0 - scan all serial numbers
    isa_pnp_id search_id;
    int cards; // number of cards found
    isa_pnp_card card[ISAPNP_CARDS]; // cards found
} wd_isapnp_scan_cards_t;

Comments of XXX

Comments of XXX should be <keywords>: <comment>. multiple names should be separated with a '/':

XXX: derry: move to compat
XXX derry: ANDROID move to compat
XXX derry: arik: move to compat

XXX: move to compat
XXX derry: move to compat
XXX derry ANDROID: move to compat
XXX derry/arik: move to compat

for or while loop

common sense should come into play when deciding whether to use for or while loops

i = 0;
while (i<10)
{
     ...
     i++;
}

for (i = 0; i<10; i++)
    ...;

for (p = p->next; p; p = p->next)
    ...;

while (p = p->next)
   ...;

while for if without a statement

while(pop_first(list))
    ;

while(pop_first(list));

for(i=0; i<*p; i++)
    ;

for (i=0; i<*p; i++);

if (a>b+10)
    ;
else if (a>b+5)
    do_x();
else if (a>b+2)
    ;
else
    do_y();

if (a>b+10);
else if (a>b+5)
    do_x();
else if (a>b+2);
else
    do_y();

Spacing

One space after assignment

put one space before and after an assignment.

int a=0;

x= x+1;
bits|=BIT5;

int a = 0;

x = x+1;
bits |= BIT5;

Assignment in for loop

if it is in a for loop, you can skip the spaces. if you skip one of the spaces, skip both

for (i= 0; i; i--)
    printf("%d", i);

for (i=0; i; i--)
    printf("%d", i);

for (i = 0; i; i--)
    printf("%d", i);

No space before statement seperator

don't put a space before a statement seperator, put one after it

for (i=0 ;i ;i--, j*=2) ;

for (i=0; i; i--, j*=2);

for (i = 0; i; i--, j *= 2);

No space after increment/decrement

don't put a space after increment and decrement. increment after the value, not before.

i --;
++j;

i--;
j++;

Don't increment, set or change value of variable inside a function call

calc_x_y(i++, 2);

// if i is not a global:
calc_x_y(i, 2);
i++;

// if i is a global that calc_x_y() uses:
i++;
calc_x_y(i-1, 2);

if (i++)
    a[i++] = 4;

set_x(i=3);

// if i is not a global:
i = 3;
set_x(i);

for loops assign inside a statement

in for loops, when there is a function that gets the next element, it should be done once (inside the step condition):
assign inside a statement: 

for (i = 0, result = get_char(); result=='\r'; result = get_char(), i++)
    handle_result(result);

for (i = 0; (result = get_char())=='\r'; i++)
    handle_result(result);

truth value in if/for/while

when assigning in a truth value (if for while), use parentheses. This helps eliminating compilation warnings.

for (i = 0; result = get_result(); i++)
    handle_result(result);
if (x = compute_num())
    return x;

for (i = 0; (result = get_result()); i++)
    handle_result(result);
if ((x = compute_num()))
    return x;

string initialization and zero termination

in string initialization and zero termination, you must use 0

string[0] = '\0';
if (string[3]=='\0')
    ...

*string = 0;
if (!string[3])
    ....

function return values

  • in functions returning a pointer, return NULL on fail.
  • in functions that only have success/fail, use 0 for success and -1 for failure (instead of TRUE and FALSE).
  • in functions with many error reasons, use negative values for the reason.
  • functions returning and integer that is "small" and positive, return -1 if fails. if 0 is an invalid success value, then you may choose to return 0 for failure.
  • in functions that can return any integer, you have two options:
    • if its error is not important, then check if you can logically return 0 or anything else that will bring desired behaviour.
    • if the error indication is important, then add another parameter to pass the error.
  • functions that return boolean values (is_big, is_directory, is_download) should return 0 for false and non 0 for true, according to standard K&R C. (not to use FALSE and TRUE). 
int do_load(void)
{
    if (ioctl(image_fd, SIOC_LOAD, 1)<0)
        return FALSE;
    return TRUE; // action executed
}

int do_load(void)
{
    if (ioctl(image_fd, SIOC_LOAD, 1)<0)
        return -1;
    return 0; // action executed
}

int is_loaded(void)
{
    if (in_memory_count>=1)
        return TRUE;
    return FALSE;
}

int is_loaded(void)
{
    if (in_memory_count>=1)
        return 1;
    return 0;
}

int is_loaded(void)
{
    return in_memory_count>=1;
}

return ok ? (device_t *) ptr : NULL;

#define EFLASH_READ 1
#define EFLASH_WRITE 2
return -EFLASH_WRITE;

implement error handling only once

implement error handling only once, by setting all variables to zero at the begining, and performing a goto to exit the function. If no error handling is required (i.e. no memory to release, no files to close, etc) - plain return should be used 

device_t *open_device(char *dev_name)
{
    device_t *dev;
    file *fp;

    dev = malloc(sizeof(*dev));
    if (!dev)
        return null;
    bzero(*dev);
    dev->name = strdup(dev_name);
    if (!dev->name)
    {
        free(dev);
        return null;
    }
    dev->fp = fopen(dev->name);
    if (!dev->fp)
    {
        free(dev->name);
        free(dev);
        return null;
    }
    fp = fopen(std_name)
    if (!fp)
    {
        fclose(dev->fp);
        free(dev->name);
        free(dev);
        return null;
    }
    fclose(fp);
    return dev;
}

device_t *open_device(char *dev_name)
{
    device_t *dev;
    file *fp = null;

    dev = malloc(sizeof(*dev));
    if (!dev)
        return NULL; // no error handling required
    BZERO(*dev);
    dev->name = strdup(dev_name);
    if (!dev->name)
        goto Error;
    dev->fp = fopen(dev->name);
    if (!dev->fp)
        goto Error;
    fp = fopen(std_name)
    if (!fp)
        goto Error;

    goto Exit;

Error:
    if (dev->name)
        free(dev->name);
    if (dev->fp)
        fclose(fp);
    free(dev);
    dev = NULL;
Exit:
    if (fp)
        fclose(fp);
    return dev;
}

Do not use goto for anything other then error handling.

    for (cnt = 0; cnt < sizeof(index_names)/sizeof(char *); cnt++)
    {
        struct stat sb;
        strcpy(index_name + filename_len, index_names[cnt]);

        // Check if the index file exists
        if (stat(index_name, &sb)>=0)
        {
            ...
            hc->sb = sb;
            goto Next;
        }
    }
    return http_dir_handler(hc);
}
Next:
....

    for (cnt = 0; cnt < sizeof(index_names)/sizeof(char *); cnt++)
    {
        struct stat sb;
        strcpy(index_name + filename_len, index_names[cnt]);

        // Check if the index file exists
        if (stat(index_name, &sb)>=0)
        {
            ...
            hc->sb = sb;
            break;
        }
    }
    if (S_ISDIR(hc->sb.st_mode))
        return http_dir_handler(hc);
}
....

naming conventions

code will be in UNIX notation. UNIX names should not include the data type, rather the meaning of the information in the data.

int iCount;
char *szName;

int is_completed;
int8 data;
int16 vendor_id;
uint count;
void *buffer;
char *new_name;
char *name;
typedef struct {...} device_t;
int read_data_block(void);
void pci_detection(void);

checking function return values

functions that return int or a pointer will be checked without comparing, if they return 0 or non 0.

if (strcmp(str, "exit")==0)
    run_app();

if (!strcmp(str, "exit"))
    run_app();

if (quit_app()==TRUE)
    SendMessage(WM_QUIT);

if (quit_app())
    SendMessage(WM_QUIT);

fp = fopen("test");
if (fp==NULL)
    return NULL;

fp = fopen("test");
if (!fp)
    return NULL;

char *buffer = locate_buffer();
if (buffer!=NULL)
    fill_buffer(buffer);

char *buffer = locate_buffer();
if (buffer)
    fill_buffer(buffer);

static functions when possible

functions

when possible use static when declaring functions which are only in scope for the file they're in. functions which are not declared static are considered a part of the API. if init() is the only function calling calc_init_time() then: 

int init(void)
{
    return calc_init_time();
}

int calc_init_time()
{
    return 5;
}

int init(void)
{
    return calc_init_time();
}

static int calc_init_time()
{
    return 5;
}

variables

// inside c file
struct cmd {
    char *name;
    int param_num;
};

struct cmd commands[] = {
     {"init", 1},
     {"shutdown", 0},
};

// inside c file
struct cmd {
    char *name;
    int param_num;
};

static struct cmd commands[] = {
     {"init", 1},
     {"shutdown", 0},
};

allocating

when allocating or using a structre, use ZALLOC/calloc or BZERO its elements.
try using BZERO instead of memset(). 

dev_t *dev = malloc(sizeof(dev_t));
locate_t locate;
memset(dev, 0, sizeof *dev);
memset(&locate, 0, sizeof locate);

dev_t *ZALLOC(dev);
locate_t locate;
if (!dev)
    goto Error;
BZERO(locate);

Check constructor initing

perfer to check constructor initing in same line of calling the constructor. this allows variable re-use, and makes it more obvious the constructor may fail. 

// GOOD but not prefered:
FILE *fp;
if (!(fp = fopen("data"))
    goto Exit;

// better:
FILE *fp = fopen("data");
if (!fp)
    goto Exit;

// GOOD but not prefered:
char *s = strstr(list, "hello");
if (!s)
    ...;

// better:
char *s;
if (!(s = strstr(list, "hello")))
    ...;

macros

functions instead of macros

try using functions instead of macros. in most cases there is no performance problem.

#define CONVERT_TIME_TO_MILLI(t) \
    (((t)->days*24*3600 + (t)->sec) * 1000)

int convert_time_to_milli(time_struct *t)
{
    return (t->days*24*3600 + t->sec) * 1000;
}

macro names

macro names should normally be all upper case, with normal spacing as rest of code

#define CONVERT_TIME_TO_MILLI( t )  (((t)->days*24*3600 + (t)->sec) * 1000)
#define CONVERT_TIME_TO_MILLI(t) \
        (((t)->days*24*3600 + (t)->sec) * 1000)

#define CONVERT_TIME_TO_MILLI(t) (((t)->days*24*3600 + (t)->sec) * 1000)

#define CONVERT_TIME_TO_MILLI(t) \
    (((t)->days*24*3600 + (t)->sec) * 1000)

macro multi arguments

using ## for macro multi arguments (x...) will result in a compilation error when using gcc 3.3.x and up

#define IOW(func,a...) wfs_##func(##a)

#define IOW(func,a...) wfs_##func(a)

macros with more than one statement

macros that are more than one statement long should be encapsulated by do-while(0). this enables using them inside if-else statements:

// example usage:
if (cond)
    DO_A_AND_B;
else
    do_something_else();

#define DO_A_AND_B() \
{ \
    do_a(); \
    do_b(); \
}

#define DO_A_AND_B() \
do { \
    do_a(); \
    do_b(); \
} while (0)

static functions in header instead of macros

if you need a function, but you don't want a library to link with, then you can use static functions in the header file instead of macros

#define CREATE_HANDLE() \
    (handle_t h, h = wd_open(), h==-1 ? NULL : h)

static handle_t create_handle(void)
{
    handle_t h = wd_open();
    if (h==-1)
        return NULL;
    return h;
}

static handle_t create_handle(void)
{
    handle_t h;
    if ((h = wd_open())==-1)
        return NULL;
    return h;
}

Don't use forward declarations

don't use forward declarations unless they are necessary. Instead change the order so the caller is below the callee.

void callee(...);
void caller(...)
{
    callee();
}
void callee(...)
{
}

void callee(...)
{
}
void caller(...)
{
    callee();
}

Prefer native c data types instead of OS specific data types.

BYTE b;
DWORD w;

unsigned char b;
unsigned long b;

Double inclusion protection

at the begining of a file, add double inclusion protection, where the file path and name is surrounted by two underscores before and after; a slash and a dot are translated to an underscore. 

#ifndef __UTIL_ZERR_H__
#define __UTIL_ZERR_H__

// the pkg/util/zerr.h content comes here

#endif

Files terminate

files should terminate with a carriage return
reason: compilers can ignore lines that are not terminated by a CR.

#endif<EOF>

#endif
<EOF>

Nested #if

nested #if directives should not be indented (neither the # character nor the directive itself, regardless of how complex they are).

#include <stdio.h>
#ifdef UNIX
#include <unistd.h>
#else
#include <windows.h>
#ifdef WINCE
#include <wince.h>
#endif
#endif

#include <stdio.h>
#ifdef UNIX
    #include <unistd.h>
#else
    #include <windows.h>
    #ifdef WINCE
        #include <wince.h>
    #endif
#endif

void sleep_ms(int ms)
{
#ifdef WINDOWS
    Sleep(ms);
#else
    sleep(ms);
#endif
}

void sleep_ms(int ms)
{
    #ifdef WINDOWS
        Sleep(ms);
    #else
        sleep(ms);
    #endif
}

#endif comments

You can put an #endif a comment telling to what if it belongs if there is a large gap between the #if and its corresponding #endif.

#ifdef WINDOWS
int htonl(int i)
{
    return (i << 16) & 0xffff0000 | (i >> 16) & 0xffff;
}
#endif // WINDOWS

#ifdef WINDOWS
int htonl(int i)
{
    return (i << 16) & 0xffff0000 | (i >> 16) & 0xffff;
}
#endif

#ifdef CONFIG_RG_FIREWALL
// Lots of firewall code, that you really have to scroll down
// to see all of it.
#endif // CONFIG_RG_FIREWALL

enum

enum should be in the same line if they are very short

enum {STATUS_OK=DEVIOCTL_NOERROR, STATUS_FAIL=-1};

enum {
    CMD_NONE=0,
    CMD_END=1,
    WP_WORD=14,
};

enum {
    CMD_NONE = 0,
    CMD_END = 1,
    WP_WORD = 14,
};

enums should have values attached

enum {
    DO_RESET = 1,
    DO_LOAD,
    DO_SAVE,
};

enum {
    DO_RESET = 1,
    DO_LOAD = 2,
    DO_SAVE = 3,
};

enum {
    DO_RESET = 10,
    DO_LOAD = 20,
    DO_SAVE = 30,
};

all enum lines be comma terminated

all enum lines be comma terminated - this makes the enum extensible.
only separate the last line, if it is meant to never be surpassed.

enum {
    DO_RESET = 1,
    DO_LOAD = 2,
    DO_SAVE = 3
};

enum {
    DO_RESET = 1,
    DO_LOAD = 2,
    DO_SAVE = 3,
};

enum {
    BANDIT1 = 1,
    BANDIT2 = 2,
    ALI_BABA = 41
};

Spaces only after commas

enum in same line should have spaces only after commas

enum { DO_RESET = 1, DO_LOAD = 2, DO_SAVE = 3 };

enum {DO_RESET=1, DO_LOAD=2, DO_SAVE=3};

Put the pointer next to the variable

char* strrev(char* s);

char *strrev(char *s);

Dynamic allocation only when needed

use stack allocation for local variables of const size. use dynamic allocation only when needed.

type_t *o = malloc(sizeof(*o));
...
free(o);

type_t o;

type_t *ZALLOC(o);
...
free(o);

// if possible:
type_t o = {0};

type_t o;
MZERO(o);

defined()

only use the defined() macro when you have a complex expression or in #elif.
If you find that you do need to use the defined() macro for more than one flag, see if the 2 flags can be grouped under another one new flag. 

#ifdef CONFIG_CRAMFS_FS
#ifndef CONFIG_ARCH_IXP22X
    cramfs_init();
#endif
#endif

#if defined(CONFIG_CRAMFS_FS)
    cramfs_init();
#endif

#ifdef CONFIG_CRAMFS_FS
    cramfs_init();
#endif

// since you really need both flags:
#if defined(CONFIG_CRAMFS_FS) || defined(CONFIG_NFS_FS)
    cramfs_to_nfs_copy();
#endif

#ifdef CONFIG_CRAMFS_FS
    cramfs_init();
#elif defined(CONFIG_NFS_FS)
    nfs_init();
#endif

#ifdef SPECIFIC_OS order

how to do and #ifdef SPECIFIC_OS what comes first, the right order is

#ifdef LINUX
    .....
#elif defined(SOLARIS)
    .....
#elif defined(VXWORKS)
    .....
#elif defined(OS2)
    .....
#elif defined(WIN95)
    .....
#elif defined(WINNT)
    .....
#elif defined(WINCE)
    .....
#endif

C++ data members names

in C++ classes give data members names starting with m_

class find_t
{
    int m_index;
    char *m_text;
};

functions

functions used in one C file: should be implemented static, and a declaration in the C file should only be added if used before implemented. (example: queue_locate_any()).
functions used out of the C file ('global'): should be declared in the H file. (example: queue_locate_item() and queue_activate_operation()). structures used in one C file: should be declared inside the C file. (example: sched_operation_t).
a pointer to the structure is needed out of the C file: a forward declaration should be in the H file, and the full decleration in the C file. (example: sched_queue_t).
the structure's members are needed out of the C file: declare it in the H file. (no example below). 

// in H file
typedef struct sched_queue_t sched_queue_t;
sched_queue_t *queue_locate_item(char *name);
void queue_activate_operation(sched_queue_t *sched);

// in C file
typedef struct {
    char *operation;
    char *user;
} sched_operation_t;
sched_operation_t valid_operations[] = {
    {"ls", "root"},
    {"rm", "anonymous"},
    {NULL, NULL}
};
struct sched_queue_t {
    struct sched_queue_t *next;
    char *name;
    char *operation;
    int expire;
};
sched_queue_t *the_queue;
static sched_queue_t *queue_locate_any(char *name)
{
    sched_queue_t *search;
    for (search = the_queue; search && strcmp(search->name, name);
        search = search->next);
    return search;
}

sched_queue_t *queue_locate_item(char *name)
{
    sched_queue_t *search = queue_locate_any(name);
    // check if already expired
    if (expire>get_current_time())
    {
        queue_remove(search);
        return NULL;
    }
    return search;
}

object oriented naming

the data structure should be the name of the object with _t at the end.
functions should all start with the object name, then operation starting with a verb. 

typedef struct {
    the_objects_data_members;
} xxx_t;

// use _alloc and _free if it only allocates memory, and does not do
// "active" operations
xxx_t *xxx_alloc(initialization_param);
void xxx_free(xxx_t *xxx);

// use _open and _close if it does active stuff like call many functions,
// register events, IOCTLs etc...
xxx_t *xxx_open(initialization_param);
void xxx_close(xxx_t *xxx);

// use _init and _uninit if it does not allocate the xxx_t structure, only
// its sons - i.e. xxx_alloc can be a simple zalloc() while xxx_init
// can be a simple ZEROM(). xxx_uninit frees data structures that the xxx_t
// points to, without freeing xxx_t itself
void xxx_init(xxx_t *xxx, initialization_param);
void xxx_uninit(xxx_t *xxx);

// methods on the xxx object
char *xxx_get_by_index(xxx_t *xxx, int index);
int xxx_get_by_name(xxx_t *xxx, char *name);
char **xxx_get_name_list(xxx_t *xxx);
void xxx_set(xxx_t *xxx, char *name);
void xxx_set_nofail(xxx_t *xxx, char *name);

in_addr dev_if_ip_get(dev_if_t *dev);

// operation ('get_ip') must start with a verb ('get')
in_addr dev_if_get_ip(dev_if_t *dev);

linked lists

simple linked lists always have a 'next' variable as the first structure member.

functions for list are xxx_free() and xxx_list_free()

the xxx_yyy (xxx is object name and yyy is function name), for functions on the object, NOT on list of objects.
the xxx_list_yyy is for functions on the list of objects (such as xxx_list_add, xxx_list_remove). 

// sample functions:
xxx_t *xxx_dup(xxx_t *p); // retunes a duplicated object
void xxx_copy(xxx_t *dst, xxx_t *src); // copy object's data
void xxx_free(xxx_t *p); // free the object
xxx_t *xxx_alloc(init_data....); // creates a new initialized object
void xxx_init(xxx_t *p, init_data....); // initializes an object
void xxx_uninit(xxx_t *p, init_data....); // un-initializes an object
// frees the list. list now is destroyed
void xxx_list_free(xxx_t *xxx_list);
// adds an object to the list
void xxx_list_add(xxx_t **xxx_list, xxx_t *new_elem);

Don't implement add and remove functions

normally, we should not implement add and remove functions (xxx_list_add and xxx_list_remove). these should only be implemented if there is specific added value (specific code) that needs to be done in addition to adding to the list.
when scanning lists in order to add/remove elements, a pointer to pointer should be used. 

typedef struct file_t {
    struct file_t *next;
    char *name;
} file_t;

// frees a single file object
file_free(file_t *p)
{
    if (p->name)
        free(p->name);
    free(p);
}

// frees the whole list of object
file_list_free(file_t *p)
{
    while (p)
    {
        file_t *tmp = p->next;
        file_free(p);
        p = tmp;
    }
}

free() handles NULL

free() also handles NULL, so no need for 'if ()' before it

if (str)
    free(str);

free(str);

exit a function with a negative value

to exit a function with a negative value, to indicate an error, and send a log message, you may use the fact that zerr() returns negative value

// GOOD but a little long:
if (ioctl()<0)
{
    zerr_f(LERR, "failed ioctl()");
    return -1;
}

// GOOD short and neat:
if (ioctl()<0)
    return zerr_f(LERR, "failed ioctl()");

constant strings longer than one line

    printf("Hello world, this is a long string that we want to print and \
is more than 80 chars long so we need to split it");

printf("Hello world, this is a long string that we want to print"
    " and is more than 80 chars long so we need to split it");

printf("Hello world, this is a long string that we want to print "
    "and is more than 80 chars long so we need to split it");

Dont use const char *

Don't use 'const char *' (or 'char const *') in code. It is impossible to integrate code written with consts into a large system.

const char *gen_token(const char *seed);

// Return a static buffer - don't change.
char *gen_token(char *seed);

Zero parameters functions

Functions that accept zero parameters should be defined as accepting void

double get_time_ms();

double get_time_ms(void);

Compile time arrays

Compile time arrays should be terminated with a NULL (or -1 when 0 is valid) element, and not by calculating the size of the the array using sizeof.

name_val_t arr[] = {
    {"PPPoA", 1},
    {"ETHoA", 2},
};
for (i = 0; i < sizeof(arr) / sizeof(arr[0]; i++)
    elem_print(arr + i);

name_val_t arr[] = {
    {"PPPoA", 1},
    {"ETHoA", 2},
    {NULL}
};
for (i = 0; arr[i].name; i++)
    elem_print(arr + i);

Trivial

Trivial > >= < <= == != expressions should not have spaces around them.
Trivial expressions examples: a_var, array[3].var[1], sizeof(xxx), (x + 1) Pointers -> are not counted as trivial expressions for > >= < <=, since its hard to read x->y>z, while its not hard to read x->y==z. 

if (x > 5)

if (x>5)

if (x+1>5)

if (x+1 > 5)

if (f(x, y) > g(y, z))

if (f(x, y)>g(y, z))

if (x->y == 5)

if (x->y==5)

if (x->y>=5)

if (x->y >= 5)

Empty lines

Don't put empty lines between code inside functions. If you want to separate code fragments, put a comment line.

for (i=0; i<5; i++)
    printf(do_stuff);

if (x>5)
    zerr(LERR, ...);

for (i=0; i<5; i++)
    printf(do_stuff);
// check for invalid results
if (x>5)
    zerr(LERR, ...);

Templates

Module foo in pkg/bar/foo.c & foo.h. All files should follow this exact example.
#include "config.h" must be first in the *.c file to make sure the features configuration affect all the rest of the files.
Header files must be self contained, i.e. - they must be able to compile without relying on another include line to come before them.
Therefore #include "foo.h" must be immediately after to make sure foo.h compiles stand alone without any dependency on includes before it.
_BEGIN_EXTERN_C and _END_EXTERN_C allow the code to link to C++.

pkg/bar/foo.c template

// LICENSE_CODE ZON
#include "config.h"
#include "foo.h"
... put here minimal includes required by foo.c ...

... put here your code ...

pkg/bar/foo.h template

// LICENSE_CODE ZON
#ifndef __BAR_FOO_H__
#define __BAR_FOO_H__
#include "config.h"

... put here minimal includes required by foo.h ...

_BEGIN_EXTERN_C

... put here functions, structures and other definitions ...

_END_EXTERN_C
#endif

#include "foo.h"

Use #include "foo.h" for headers in the same directory as the source including them.
Use #include "path_to_foo/foo.h" for headers in other directories, where path_to_foo is relative to root pkg directory.
Use #include <foo.h> for external system files. 

#include "stdio.h"
#include <dhcp_lease.h>
#include "hash.h"

#include <stdio.h>
#include "dhcp_lease.h" // we are in pkg/dhcp directory
#include "util/hash.h" // from pkg/util/hash.h

Temporary disabling test

When temporary disabling test code that fail:
Do not indent the code of the disabled tests.

if (!running_on_valgrind) // XXX: yoni: fails on BAT
    jtest_eq(...);

if (!running_on_valgrind) // XXX: yoni: fails on BAT
jtest_eq(...);

If it is only one test

If it is only one test (one statement), then don't use { } even if the statement is written in 2 lines

if (!running_on_valgrind) // XXX: yoni: fails on BAT
{
jtest_run(xxx, yyy,
    zzz);
}

if (!running_on_valgrind) // XXX: yoni: fails on BAT
jtest_run(xxx, yyy,
    zzz);

Open { on the same if() line

// XXX: yoni: fails on BAT
if (!running_on_valgrind)
{
jtest_run(xxx, yyy, zzz);
jtest_run(xxx, yyy, zzz);
}

if (!running_on_valgrind) { // XXX: yoni: fails on BAT
jtest_run(xxx, yyy, zzz);
jtest_run(xxx, yyy, zzz);
}

Well hello there!

We're in the process of giving our website a
new look, call it a makeover if you prefer :)

Rest assured, this will be updated soon too,
we're on it.