wiki:Hacking

Version 37 (modified by zaytsev, 7 weeks ago) (diff)

--

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.

This is right:

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);

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); }

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't 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.

If variable is introduced to store intermediate value, declare it as constant.

This is right:

const vfs_path_t *vpath = vfs_path_from_str (filename);

This is wrong:

vfs_path_t *vpath;

vpath = vfs_path_from_str (filename);

Avoid initialized and not initialized variables in one declaration.

This is right:

int a;
int b = 0;

This is wrong:

int a, b = 0;

Avoid several non-trivial variable initializations in one declaration.

This is right:

int a = 2 + 5;
int b = 4 * 3 - 1;

This is wrong:

int a = 2 + 5, b = 4 * 3 - 1;

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;
    
    const ComputeDirSizeUI *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 ());
}

Avoid abusing non-const function parameters as local variables:

This is right:

void
foo (const int iterations)
{
    int result;

    result = do_one_thing (iterations);
    do_something (&result);
    ...
}

This is wrong:

void
foo (int iterations)
{
    iterations = do_one_thing (iterations);
    do_something (&iterations);
    ...
}

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