============================================================================
Freeciv Coding Style Guide
============================================================================
If you want to hack Freeciv, and want your patches to be accepted, it helps
to follow some simple style rules. Yes, some of these are a bit nit-picky,
but wars are fought over the silliest things ...
- This style is used for all code in Freeciv. Freeciv gtk-clients use
this style, not gtk style.
- Freeciv is mostly programmed in C, C89 with some C99 features.
Qt parts are programmed in C++. Even C++ parts should have mostly
consistent style with C parts, so where not otherwise noted, this
guide applies to C++ parts too. Headers that are included to both
C and C++ source files follow C style.
- C++-style comments (i.e., // comments) are forbidden in C code.
They should be used for single-line comments in C++ code.
- Declaring variables in the middle of the scope is forbidden
(unless you are using C99 dynamic arrays and you need to check the size
of the array before you declare it).
- Where variable is logically boolean, 'bool' type should be used even in
C code. To make sure that the type is defined, include utility/support.h.
In C code boolean values are uppercase macros 'TRUE' and 'FALSE'.
In C++ code lowercase boolean values 'true' and 'false' should be used.
- Functions that take no arguments should be declared and defined with
'void' argument list in C code, and empty argument list in C++ code.
C:
int no_arguments(void);
C++:
int no_arguments();
- Use K&R indentation style with indentation 2 (if in doubt, use "indent -kr
-i2 -l77", but beware that that will probably mangle the _() macros used
to mark translated strings and the brace style for iteration macros).
- Do not re-indent areas of code you are not modifying or creating.
- Here are the most important formatting rules:
- Lines are at most 77 characters long, including the terminating newline.
- The tab width is 8 spaces for tabs that already exist in the source code
(this is just so that old code will appear correctly in your editor).
However, tabs should be avoided in newly written code.
- The indentation is 2 spaces per level for all new code; do not use tabs
for any kind of indentation. The one exception to this is if you are
just adding one line to a set of lines that are already indented with
some strange mix of tabs and spaces; in this case you may use the same
indentation as the surrounding lines.
- Do not add more than 2 empty lines between any sections in the code.
- Spaces are inserted before and after operators: instead of "int a,b,c;"
use "int i, j, k;" and instead of
if(foo<=bar){
c=a+b;
}
use
if (foo <= bar) {
c = a + b;
}
Note the space between "if" and the bracket.
- Switch statement case labels are aligned with the enclosing "switch":
switch (value) {
case MY_VALUE1:
do_some_stuff(value);
break;
case MY_VALUE2:
{
int value2 = value + 5;
do_some_other_stuff(value2);
}
break;
}
- In the rare case that you actually use goto, the label should be all
capitals and "out-dented" in the block in which it is contained:
static int frob(int n)
{
int i, j;
for (i = 0; i < n; i++) {
for (j = i; j < n; j++) {
if (some_condition(i, j)) {
goto EXIT_LOOP;
}
}
}
EXIT_LOOP:
return 123;
}
- If a function prototype exceeds 77 characters on one line, you should
put the return value type and storage specifier on the line immediately
above it:
static const struct incredibly_long_structure_name *
get_a_const_struct_incredibly_long_structure_name(int id);
- If arguments in a function prototype or a function call cause the line
to exceed 77 characters, they should be placed on the following line and
lined up with spaces to the column after the '(':
void some_function(const struct another_really_long_name *arln,
int some_other_argument);
- If the line is still too long for some reason, you may place the
arguments two indentation levels on the next line:
a_very_awkward_long_function_name(some_argument,
"A really long string that would have to be cut up.");
But you should try to avoid this situation, either by naming your
functions/types/variables more succinctly, or by using helper variables
or functions to split the expression over a number of lines.
- An empty line should be placed between two separate blocks of code.
- Place operators at the beginning of a line, not at the end. It should be
if ((a
&& b)
|| c) {
instead of
if ((a &&
b) ||
c) {
============================================================================
Comments
============================================================================
- All comments should have proper English grammar, spelling and punctuation,
but you should not capitalize names of identifiers (variables, types,
functions, etc.) used in the code. If using plain identifiers in sentences
would be confusing to the reader, you should put the names in quotes.
- Every function should have a comment header. The comment should look like
the example below, indented by two spaces. It should be above the
function's implementation, not the prototype:
/****************************************************************************
The description of the function should be here. Also describe what is
expected of the arguments if it is not obvious. Especially note down any
non-trivial assumptions that the function makes.
Do _not_ introduce a new function without some sort of comment.
****************************************************************************/
int the_function_starts_here(int value)
{
return value + 2;
}
- One line comments should be indented correctly and placed above the code
being commented upon:
int x;
/* I am a single line comment. */
x = 3;
- For multiline comments, asterisks should be placed in front of the comment
line like so:
/* I am a multiline
* comment, blah
* blah blah. */
- If you need to comment a declared variable, it should be as such:
struct foo {
int bar; /* bar is used for ...
* in ... way. */
int blah; /* blah is used for ... . */
};
Or if the comment is very long, it may be placed above the field
declaration, as in the one-line or multi-line comment cases.
- Comments in conditionals: if you need a comment to show program flow, it
should be below the if or else:
if (is_barbarian(pplayer)) {
x++;
} else {
/* If not barbarian... */
x--;
}
- Comments to translators are placed before the N_(), _(), Q_() or PL_()
marked string, and are preceded by "TRANS:". . They must be on the same or
immediately previous line to the gettext invocation. These comments are
copied to the translator's file. Use them whenever you think the
translators may need some more information:
/* TRANS: Do not translate "commandname". */
printf(_("commandname <arg> [-o <optarg>]"));
============================================================================
Declaring Variables
============================================================================
- Avoid static and global variables if at all possible. When you absolutely
do need them, minimize the number of times they are referenced in the code
(e.g. use a helper function to wrap their access).
- Never initialize variables with values that make no sense as their
value in case they get used. If there's no sensible initialization
value for a variable, leave it uninitialized. This allows various
tools to detect if such a variable ever gets used without assigning
proper value to it.
- Variables can be initialized as soon as they are declared:
int foo(struct unit *punit)
{
int x = punit->x;
int foo = x;
char *blah;
/* Etc. */
(But you should generally check arguments to functions before using them,
unless you are absolutely sure that pointers are not NULL, etc.)
- After variables are declared, there should be an empty line before the
rest of the function body.
- Merging declarations: variables do not have to be declared one per line;
however, they should only be grouped by similar function.
int foo(struct city *pcity)
{
int i, j, k;
int total, cost;
int build = pcity->shield_stock;
}
- When declaring a pointer, there should be a space before '*' and no space
after, except if it is a second '*'.
struct unit *find_random_unit(struct unit **array, size_t num)
{
struct unit *const *prand = array + fc_rand(num);
return *prand;
}
instead of
struct unit* find_random_unit(struct unit* *array, size_t num)
{
struct unit * const* prand = array + fc_rand(num);
return *prand;
}
============================================================================
Bracing
============================================================================
- Function braces begin and end in the first column:
int foo(void)
{
return 0;
}
instead of
int foo(void) {
return 0;
}
- Use extra braces for iteration macros. Note that the "*_iterate_end;"
should be placed on the same line as the end brace:
unit_list_iterate(pcity->units_supported, punit) {
kill(punit);
} unit_list_iterate_end;
- In switch statements, braces should only be placed where needed, i.e. to
protect local variables.
- Braces shall always be used after conditionals, loops, etc.:
if (x == 3) {
return;
}
and
if (x == 3) {
return 1;
} else {
return 0;
}
not
if (x == 3)
return 1; /* BAD! */
============================================================================
Enumerators
============================================================================
- First of all, reread comment about the switch statement indentations and
braces.
- Avoid the usage of magic values (plain hard-coded value, such as 0 or -1)
and prefer the usage of enumerators. If an enumeration cannot be defined
for any reason, then define a macro for this value.
- Avoid storing magic values in external processes. For example, savegames
shouldn't contain any enumerators as magic numbers. They should be saved
as strings, to keep compatibility when their definition is changed. For
doing this, there are some tools in utility/specenum_gen.h; have a look at
it.
- Avoid the usage of the default case in switch statements, if possible. The
default case removes the warning of the compiler when a value is missing
in a switch statement.
============================================================================
Including Headers
============================================================================
- Order include files consistently: all includes are grouped together.
These groups are divided by an empty line. The order of these groups is as
follows:
1) fc_config.h (see below)
2) system include files which are OS-independent (part of C-standard or
POSIX)
3) system include files which are OS-dependent or conditional includes
4) include files from utility/
5) include files from common/
6) include files from client/
7) include files from server/ and ai/
8) include the header corresponding to the current c source file after
all other headers.
Each group is sorted in alphabetic order. This helps to avoid adding
unnecessary or duplicated include files.
Always set a comment to determine the location of the following headers
before every group.
It is very important that '#include <fc_config.h>' is at the top of
every .c file (it need not be included from .h files). Some definitions in
fc_config.h will affect how the code is compiled, without which you can end
up with bad and untraceable memory bugs.
#ifdef HAVE_CONFIG_H
#include <fc_config.h>
#endif
#include <stdlib.h>
/* utility */
#include "log.h"
/* common */
#include "game.h"
#include "myfileheader.h"
- For headers within a subdirectory path, the common rule is to set them
in an additional group, after the same group (don't forget the location
comment).
/* common */
#include "game.h"
/* common/aicore */
#include "pf_tools.h"
However, there is an exception to this. The last group is always the one
we are working on. So, if we are working on the common part, the order
should be:
/* common/aicore */
#include "pf_tools.h"
/* common */
#include "game.h"
Same observations with ai/ and server/. When working on the server/
directory, the order should be:
/* ai */
#include "aitools.h"
/* server */
#include "srv_main.h"
and working on the ai/ directory:
/* server */
#include "srv_main.h"
/* ai */
#include "aitools.h"
- Do not include headers in other headers if at all possible. Use forward
declarations for pointers to structs:
struct connection;
void a_function(struct connection *pconn);
instead of
#include "connection.h"
void a_function(struct connection *pconn);
- Of course, never include headers of non-linked parts of the code. For
example, never include client/ headers into a server/ file. Also, in the
client/ directory, GUI specific headers are never included. Always, use
the common set of headers defined in client/include/.
============================================================================
Object-Oriented Programming
============================================================================
Freeciv is not really object-oriented programming, but last written parts
seems to tend to there. Also, there are more and more parts which are
modular, so there are some observations to do:
- Any function or member of a module must be prefixed by the name of this
module, or an abbreviation of it (but use the same prefix for all members
please!). Never set the module name as suffix!
/* Super mega cool module! */
void smc_module_init(void);
void smc_module_free(void);
not
/* Super mega cool module! */
void smc_module_init(void);
void sm_cool_free(void);
neither
/* Super mega cool module! */
void init_smc_module(void);
void free_smc_module(void);
- A function which allocates memory for a pointer variable should use the
suffix '_new'. The memory is freed by a corresponding function with the
suffix '_destroy'.
{
struct test *t = test_new();
/* Do something. */
test_destroy(t);
}
- The suffix '_init' should be used for functions which initialize some
static data. The name of the corresponding function to deinitialize stuff
should use the suffix '_free' (see server/settings.c or common/map.c).
{
struct astring str;
astr_init(&str);
/* Do something. */
astr_free(&str);
}
============================================================================
Miscellaneous
============================================================================
- If an empty statement is needed, you should put an explanatory comment
in an empty block (i.e. {}):
while (*i++) {
/* Do nothing. */
}
- Use the postfix operator instead of the prefix operator when either will
work. That is, write "a++" instead of "++a".
- Strive to make your code re-entrant (thread/recursion safe), as long as
this does not make the implementation too cumbersome or involved.
- Strive to make your code modular: make it independent from other parts of
the codebase, and assume as little as possible about the circumstances in
which it is used.
- Strive to avoid code duplication: if some part of the code is repeated in
several places, factor it out into a helper function.
- Try to use static inline functions and const data instead of macros.
- If helper functions internal to freeciv are added, prefix their names
with 'fc_'. Do not use 'my_' because it is also used by MySQL and could
be included in some libs.
- Do not use assert() or die(); instead use the macros defined within
utility/log.h:
fc_assert(condition)
fc_assert_ret(condition)
fc_assert_ret_val(condition, val)
fc_assert_action(condition, action_on_failure)
fc_assert_exit(condition, action_on_failure)
fc_assert_msg(condition, message, ...)
fc_assert_ret_msg(condition, message, ...)
fc_assert_ret_val_msg(condition, val, message, ...)
fc_assert_action_msg(condition, action_on_failure, message, ...)
fc_assert_exit_msg(condition, action_on_failure, message, ...)
This way error conditions can be handled gracefully while still enforcing
invariants you expect not to be violated in the code.
(By default execution will continue with a warning, but it can be made
to halt by specifying the '-F' option to the client or server.)
int foo_get_id(const struct foo *pfoo)
{
fc_assert_ret_val(pfoo != NULL, -1);
return pfoo->id;
}
- Do not put multiple conditions in the same fc_assert*() statement:
fc_assert(pfoo != NULL);
fc_assert(pfoo->id >= 0);
instead of
fc_assert(pfoo != NULL && pfoo->id >= 0);
- Never include functionality also otherwise necessary inside fc_assert*().
Such functionality would be missing from release builds where asserts
are disabled. If you want to assert return value of a critical function
call, make the call outside assert and place the return value to variable
and then assert value of that variable.
- For strings containing multiple sentences, use a single space after periods
(not two, not zero, just one).
- If you use a system specific feature, do not add #ifdef __CRAY__ or
something like that. Rather write a check for that feature for
configure.ac, and use a meaningful macro name in the source.
- Always prototype global functions in the appropriate header file. Local
functions should always be declared as static. To catch these and some
other problems please use the following warning options "-Wall
-Wpointer-arith -Wcast-align -Wmissing-prototypes -Wmissing-declarations
-Wstrict-prototypes -Wnested-externs -Wl,--no-add-needed" if you use gcc.
- Always check compilation with the configure option --enable-debug set.
- Header files should be compatible with C++ but code (.c) files need not
be. This means some C++ keywords (like "this" or "class") may not be used
in header files. It also means casts on void pointers (which should be
avoided in C files) must be used in headers.
- To assign null pointer, or to compare against one, use 'NULL' in C code,
and 'nullptr' in C++ code.
- If you send patches, use "diff -u" (or "diff -r -u"). "svn diff" works
correctly without extra parameters.
For further information, see:
<http://www.freeciv.org/wiki/How_to_Contribute>.
Also, name patch files descriptively (e.g. "fix-foo-bug-0.patch" is good,
but "freeciv.patch" is not).
- When doing a "diff" for a patch, be sure to exclude unnecessary files by
using the "-X" argument to "diff". E.g.:
% diff -ruN -Xdiff_ignore freeciv_svn freeciv >/tmp/fix-foo-bug-0.patch
A suggested "diff_ignore" file is included in the Freeciv distribution.
============================================================================
