wiki:Hacking
Last modified 7 months ago Last modified on 11/17/16 17:43:47

HACKING

Coding Style

  • Follow source templates (see maint/templates in the source tree).
  • Maximum line width is 100 characters. The reason is not about people with low-res screens but rather sticking to 100 columns prevents you from easily nesting more than one level of if statements or other code blocks.
  • No tabs. Use 4 space tabs with whitespace fillers to indent.
  • No trailing whitespace.
  • Follow the GNU-Style guidelines.

To format the code, use the indent utility with following options:

indent \
  --gnu-style \
  --format-first-column-comments \
  --indent-level4 \
  --brace-indent0 \
  --line-length100 \
  --no-tabs \
  --blank-lines-after-procedures 

or in short notation:

indent -gnu -fc1 -i4 -bli0 -nut -bap -l100

or just run

make indent

Programming Tips

Comments

Comments should always use the standard C syntax. C++ style comments are not currently allowed.

Lines before a comment should be empty. If the comment directly belongs to the following code, there should be no empty line after the comment, except if the comment contains a summary of multiple following code blocks.

This is right:

/*
 * This is a multi line comment,
 * Delete '\n' char.
 * Note that edit_delete() will not corrupt anything if called while
 *   cursor position is EOF.
 */
(void) edit_delete (edit);

/*  This is a one line comment. Allocate additional memory.  */
mem = (char *) malloc (memneed);

/**
 * @brief This is a doxygen comment.
 *
 * This is a more detailed explanation of
 * this simple function.
 *
 * @param[in]   param1     The parameter value of the function.
 *
 * @param[out]  result1    The result value of the function.
 *
 * @return              0 on success and -1 on error.
 */
int example (int param1, int *result1);

This is wrong:

//This is a one line comment.

/*This is a one line comment.*/

/* This is a multi line comment,
   with some more words...*/

Conditions in code blocks (if, for, case...)

Always follow an 'if' keyword with a space but don't include additional spaces following or preceding the parentheses in the conditional.

This is right:

if (i == 0)

This is wrong:

if ( i == 0 )

if (0 == i)

Functions usage

Always insert a space between the name and left parentheses when invoking functions.

This is right:

do_example (int param1, int *result1);

This is wrong:

do_example(int param1, int *result1);

Braces

Braces for code blocks used by for, if, switch, while, do..while, etc. should begin on the next line after the statement keyword and end on a line of their own.

Functions are different and the beginning left brace should be located in the first column on the next line.

If the beginning statement has to be broken across lines due to length, the beginning brace should be on a line of its own.

Do not unnecessarily use braces where a single statement will do. If lines is more than one to use braces.

This is right:

if (xterm_flag && xterm_title)
{
    path = strip_home_and_password (current_panel->cwd);
    ...
}

for (j = 0; j < 10; j++)
{
    for (i = 0; str_options[i].opt_name != NULL; i++)
        g_free (*str_options[i].opt_addr);
}

This is wrong:

if (xterm_flag && xterm_title) {
    path = strip_home_and_password (current_panel->cwd);
    ...
}

if (xterm_flag && xterm_title)
{
    path = strip_home_and_password (current_panel->cwd); }

for (k = 0; k < 10; k++)
     for (j = 0; j < 10; j++)
        for (i = 0; str_options[i].opt_name != NULL; i++)
            g_free (*str_options[i].opt_addr);

Goto

Use "goto" only when necessary. "goto"s are evil, but they can greatly enhance readability and reduce memory leaks when used as the single exit point from a function.

This is right:

{
    if (link_type == LINK_HARDLINK)
    {
        src = g_strdup_printf (_("Link %s to:"), str_trunc (fname, 46));
        dest = input_expand_dialog (_("Link"), src, MC_HISTORY_FM_LINK, "");

        if (dest == NULL || *dest == '\0')
            goto cleanup;
        ...
        ...
    }
  ...
  ...

  cleanup:
    g_free (src);
    g_free (dest);
}

Readable Code

Use your best judgement and choose the more readable option. Remember that many other persons will review it:

This is right:

bytes = read (fd, &routine.pointer, sizeof (routine));
if (bytes == -1 || (size_t) bytes < sizeof (routine))
    ...

This is wrong:

if ((bytes = read (fd, &routine.pointer, sizeof (routine))) == -1 || (size_t) bytes < sizeof (routine))
    ...

Don not place more than one statement in one line.

This is right:

a = 0;
b = 2;

a = f();
if (a == 2)
    b = 5;

This is wrong:

a = 0; b = 2;

if ((a = f()) == 2)
    b = 5;

if (a == 2) b = 5;

Use explicit comparison in equality operators:

This is right:

void *p1, *p2;
int i1, i2;
char c1, c2;

if (p1 != NULL)
if (p2 == NULL)

if (i1 != 0)
if (i2 == 0)

if (c1 != '\0')
if (c2 == '\0')

This is wrong:

void *p1, *p2;
int i1, i2;
char c1, c2;

if (p1)
if (!p2)

if (i1)
if (!i2)

if (c1)
if (!c2)

Do not check boolean values for equality:

This is right:

gboolean b1, b2;

if (b1)
if (!b2)

This is wrong:

gboolean b1, b2;

if (b1 == TRUE)
if (b2 == FALSE)

Variables

Reduce variable scope as much as possible: declare local variable in that block where it is used.

Don't mix variable declaration and code, declare variable only in the beginning of block.

Split variable declaration and code using one empty line. Avoid using function to initialize variable. Split variable declaration and value assignment.

This is right:

vfs_path_t *vpath;

vpath = vfs_path_from_str (filename);

This is wrong:

vfs_path_t *vpath = vfs_path_from_str (filename);

Helper variables

Try to avoid passing function calls as function parameters in new code. This makes the code much easier to read and it's also easier to use the "step" command within gdb.

This is right:

void
dirsizes_cmd (void)
{
    WPanel *panel = current_panel;
    int i;
    ComputeDirSizeUI *ui;

    ui = compute_dir_size_create_ui ();

    compute_dir_size_destroy_ui (ui);
    ...
    recalculate_panel_summary (panel);
}

This is wrong:

void
dirsizes_cmd (void)
{
    compute_dir_size_destroy_ui (compute_dir_size_create_ui ());
}

Headers

Do not mix headers

This is right:

#include <errno.h>
#include <stdio.h>
#include <string.h>

#include <sys/types.h>
#include <sys/stat.h>

#include "lib/global.h"
#include "lib/tty/tty.h"        /* LINES, tty_touch_screen() */
#include "lib/tty/win.h"        /* do_enter_ca_mode() */

#include "src/subshell.h"       /* use_subshell */
#include "src/help.h"           /* interactive_display() */
#include "src/setup.h"

This is wrong:

#include <errno.h>
#include <sys/types.h>
#include <stdio.h>
#include <string.h>

#include <sys/stat.h>

#include "src/subshell.h"       /* use_subshell */
#include "src/help.h"           /* interactive_display() */

#include "lib/tty/tty.h"        /* LINES, tty_touch_screen() */
#include "lib/tty/win.h"        /* do_enter_ca_mode() */

#include "src/setup.h"
#include "lib/global.h"

Use short comment for header file

This is right:

#include "lib/tty/tty.h"        /* LINES, tty_touch_screen() */
#include "lib/tty/win.h"        /* do_enter_ca_mode() */
#include "src/subshell.h"       /* use_subshell */
#include "src/help.h"           /* interactive_display() */