wiki:Hacking

Version 25 (modified by andrew_b, 9 years ago) (diff)

Templates

HACKING

Code Style

Please use the same indentation as other developers.

  • 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.
  • Use 4 space tabs with whitespace fillers to Indent. Never use tabs.
  • 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

Programming Tips

Comments

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

The 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 || !*dest)
            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)))

if (read (from_parent_fd, &i, sizeof (int)) != sizeof (int))
    return NULL;

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)

Variables

Declare variable only in the beginning of block. Split variable declaration and code using one empty line. Try to avoid using functions to initialize variables. 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);

Make use of helper variables

Please 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() */

Do not mix global and local variables and functions. Do not mix static and non-static statments.

Follow typical structure of *.c file:

/*** global variables ****************************************************************************/
/*** file scope macro definitions ****************************************************************/
/*** file scope type declarations ****************************************************************/
/*** file scope variables ************************************************************************/
/*** file scope functions ************************************************************************/
/* --------------------------------------------------------------------------------------------- */
/* --------------------------------------------------------------------------------------------- */
/*** public functions ****************************************************************************/
/* --------------------------------------------------------------------------------------------- */

(see maint/templates in the source tree).