Version 28 (modified by andrew_b, 9 years 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
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() */