1============================================================================ 2 Freeciv Coding Style Guide 3============================================================================ 4 5If you want to hack Freeciv, and want your patches to be accepted, it helps 6to follow some simple style rules. Yes, some of these are a bit nit-picky, 7but wars are fought over the silliest things ... 8 9- This style is used for all code in Freeciv. Freeciv gtk-clients use 10 this style, not gtk style. 11 12- Freeciv is mostly programmed in C, C89 with some C99 features. 13 Qt parts are programmed in C++. Even C++ parts should have mostly 14 consistent style with C parts, so where not otherwise noted, this 15 guide applies to C++ parts too. Headers that are included to both 16 C and C++ source files follow C style. 17 18- C++-style comments (i.e., // comments) are forbidden in C code. 19 They should be used for single-line comments in C++ code. 20 21- Declaring variables in the middle of the scope is forbidden 22 (unless you are using C99 dynamic arrays and you need to check the size 23 of the array before you declare it). 24 25- Where variable is logically boolean, 'bool' type should be used even in 26 C code. To make sure that the type is defined, include utility/support.h. 27 In C code boolean values are uppercase macros 'TRUE' and 'FALSE'. 28 In C++ code lowercase boolean values 'true' and 'false' should be used. 29 30- Functions that take no arguments should be declared and defined with 31 'void' argument list in C code, and empty argument list in C++ code. 32 33 C: 34 int no_arguments(void); 35 36 C++: 37 int no_arguments(); 38 39- Use K&R indentation style with indentation 2 (if in doubt, use "indent -kr 40 -i2 -l77", but beware that that will probably mangle the _() macros used 41 to mark translated strings and the brace style for iteration macros). 42 43- Do not re-indent areas of code you are not modifying or creating. 44 45- Here are the most important formatting rules: 46 47 - Lines are at most 77 characters long, including the terminating newline. 48 49 - The tab width is 8 spaces for tabs that already exist in the source code 50 (this is just so that old code will appear correctly in your editor). 51 However, tabs should be avoided in newly written code. 52 53 - The indentation is 2 spaces per level for all new code; do not use tabs 54 for any kind of indentation. The one exception to this is if you are 55 just adding one line to a set of lines that are already indented with 56 some strange mix of tabs and spaces; in this case you may use the same 57 indentation as the surrounding lines. 58 59 - Do not add more than 2 empty lines between any sections in the code. 60 61 - Spaces are inserted before and after operators: instead of "int a,b,c;" 62 use "int i, j, k;" and instead of 63 64 if(foo<=bar){ 65 c=a+b; 66 } 67 68 use 69 70 if (foo <= bar) { 71 c = a + b; 72 } 73 74 Note the space between "if" and the bracket. 75 76 - Switch statement case labels are aligned with the enclosing "switch": 77 78 switch (value) { 79 case MY_VALUE1: 80 do_some_stuff(value); 81 break; 82 case MY_VALUE2: 83 { 84 int value2 = value + 5; 85 do_some_other_stuff(value2); 86 } 87 break; 88 } 89 90 - If case of a switch is supposed to continue to the next case, 91 explicitly mark it so by using fc__fallthrough; This also avoids 92 compiler warning about missing break; when such warnings are enabled. 93 94 switch (value) { 95 case MY_VALUE1: 96 do_some_stuff(value); 97 98 fc__fallthrough; /* break; intentionally left out */ 99 case MY_VALUE2: 100 { 101 int value2 = value + 5; 102 103 do_some_other_stuff(value2); 104 } 105 break; 106 } 107 108 - In the rare case that you actually use goto, the label should be all 109 capitals and "out-dented" in the block in which it is contained: 110 111 static int frob(int n) 112 { 113 int i, j; 114 for (i = 0; i < n; i++) { 115 for (j = i; j < n; j++) { 116 if (some_condition(i, j)) { 117 goto EXIT_LOOP; 118 } 119 } 120 } 121 EXIT_LOOP: 122 return 123; 123 } 124 125 - If a function prototype exceeds 77 characters on one line, you should 126 put the return value type and storage specifier on the line immediately 127 above it: 128 129 static const struct incredibly_long_structure_name * 130 get_a_const_struct_incredibly_long_structure_name(int id); 131 132 - If arguments in a function prototype or a function call cause the line 133 to exceed 77 characters, they should be placed on the following line and 134 lined up with spaces to the column after the '(': 135 136 void some_function(const struct another_really_long_name *arln, 137 int some_other_argument); 138 139 - If the line is still too long for some reason, you may place the 140 arguments two indentation levels on the next line: 141 142 a_very_awkward_long_function_name(some_argument, 143 "A really long string that would have to be cut up."); 144 145 But you should try to avoid this situation, either by naming your 146 functions/types/variables more succinctly, or by using helper variables 147 or functions to split the expression over a number of lines. 148 149- An empty line should be placed between two separate blocks of code. 150 151- Place operators at the beginning of a line, not at the end. It should be 152 153 if ((a 154 && b) 155 || c) { 156 157 instead of 158 159 if ((a && 160 b) || 161 c) { 162 163 164============================================================================ 165 Comments 166============================================================================ 167 168- All comments should have proper English grammar, spelling and punctuation, 169 but you should not capitalize names of identifiers (variables, types, 170 functions, etc.) used in the code. If using plain identifiers in sentences 171 would be confusing to the reader, you should put the names in quotes. 172 173- Every function should have a comment header. The comment should look like 174 the example below, indented by two spaces. It should be above the 175 function's implementation, not the prototype: 176 177/**************************************************************************** 178 The description of the function should be here. Also describe what is 179 expected of the arguments if it is not obvious. Especially note down any 180 non-trivial assumptions that the function makes. 181 182 Do _not_ introduce a new function without some sort of comment. 183****************************************************************************/ 184int the_function_starts_here(int value) 185{ 186 return value + 2; 187} 188 189- One line comments should be indented correctly and placed above the code 190 being commented upon: 191 192 int x; 193 194 /* I am a single line comment. */ 195 x = 3; 196 197- For multiline comments, asterisks should be placed in front of the comment 198 line like so: 199 200 /* I am a multiline 201 * comment, blah 202 * blah blah. */ 203 204- If you need to comment a declared variable, it should be as such: 205 206 struct foo { 207 int bar; /* bar is used for ... 208 * in ... way. */ 209 int blah; /* blah is used for ... . */ 210 }; 211 212 Or if the comment is very long, it may be placed above the field 213 declaration, as in the one-line or multi-line comment cases. 214 215- Comments in conditionals: if you need a comment to show program flow, it 216 should be below the if or else: 217 218 if (is_barbarian(pplayer)) { 219 x++; 220 } else { 221 /* If not barbarian... */ 222 x--; 223 } 224 225- Comments to translators are placed before the N_(), _(), Q_() or PL_() 226 marked string, and are preceded by "TRANS:". . They must be on the same or 227 immediately previous line to the gettext invocation. These comments are 228 copied to the translator's file. Use them whenever you think the 229 translators may need some more information: 230 231 /* TRANS: Do not translate "commandname". */ 232 printf(_("commandname <arg> [-o <optarg>]")); 233 234 235============================================================================ 236 Declaring Variables 237============================================================================ 238 239- Avoid static and global variables if at all possible. When you absolutely 240 do need them, minimize the number of times they are referenced in the code 241 (e.g. use a helper function to wrap their access). 242 243- Variables should be declared in the innermost block possible, i.e., they 244 should not be visible where they are not needed. 245 246- Never initialize variables with values that make no sense as their 247 value in case they get used. If there's no sensible initialization 248 value for a variable, leave it uninitialized. This allows various 249 tools to detect if such a variable ever gets used without assigning 250 proper value to it. 251 252- Variables can be initialized as soon as they are declared: 253 254 int foo(struct unit *punit) 255 { 256 int x = punit->x; 257 int foo = x; 258 char *blah; 259 260 /* Etc. */ 261 262 (But you should generally check arguments to functions before using them, 263 unless you are absolutely sure that pointers are not NULL, etc.) 264 265- After variables are declared, there should be an empty line before the 266 rest of the function body. 267 268- Merging declarations: variables do not have to be declared one per line; 269 however, they should only be grouped by similar function. 270 271 int foo(struct city *pcity) 272 { 273 int i, j, k; 274 int total, cost; 275 int build = pcity->shield_stock; 276 } 277 278- When declaring a pointer, there should be a space before '*' and no space 279 after, except if it is a second '*'. 280 281 struct unit *find_random_unit(struct unit **array, size_t num) 282 { 283 struct unit *const *prand = array + fc_rand(num); 284 285 return *prand; 286 } 287 288 instead of 289 290 struct unit* find_random_unit(struct unit* *array, size_t num) 291 { 292 struct unit * const* prand = array + fc_rand(num); 293 294 return *prand; 295 } 296 297 298============================================================================ 299 Bracing 300============================================================================ 301 302- Function braces begin and end in the first column: 303 304 int foo(void) 305 { 306 return 0; 307 } 308 309 instead of 310 311 int foo(void) { 312 return 0; 313 } 314 315- Use extra braces for iteration macros. Note that the "*_iterate_end;" 316 should be placed on the same line as the end brace: 317 318 unit_list_iterate(pcity->units_supported, punit) { 319 kill(punit); 320 } unit_list_iterate_end; 321 322- In switch statements, braces should only be placed where needed, i.e. to 323 protect local variables. 324 325- Braces shall always be used after conditionals, loops, etc.: 326 327 if (x == 3) { 328 return; 329 } 330 331 and 332 333 if (x == 3) { 334 return 1; 335 } else { 336 return 0; 337 } 338 339 not 340 341 if (x == 3) 342 return 1; /* BAD! */ 343 344 345============================================================================ 346 Enumerators 347============================================================================ 348- First of all, reread comment about the switch statement indentations and 349 braces. 350 351- Avoid the usage of magic values (plain hard-coded value, such as 0 or -1) 352 and prefer the usage of enumerators. If an enumeration cannot be defined 353 for any reason, then define a macro for this value. 354 355- Avoid storing magic values in external processes. For example, savegames 356 shouldn't contain any enumerators as magic numbers. They should be saved 357 as strings, to keep compatibility when their definition is changed. For 358 doing this, there are some tools in utility/specenum_gen.h; have a look at 359 it. 360 361- Avoid the usage of the default case in switch statements, if possible. The 362 default case removes the warning of the compiler when a value is missing 363 in a switch statement. 364 365 366============================================================================ 367 Including Headers 368============================================================================ 369- Order include files consistently: all includes are grouped together. 370 These groups are divided by an empty line. The order of these groups is as 371 follows: 372 373 1) fc_config.h (see below) 374 2) system include files which are OS-independent (part of C-standard or 375 POSIX) 376 3) system include files which are OS-dependent or conditional includes 377 4) include files from utility/ 378 5) include files from common/ 379 6) include files from client/ 380 7) include files from server/ and ai/ 381 8) include the header corresponding to the current c source file after 382 all other headers. 383 384 Each group is sorted in alphabetic order. This helps to avoid adding 385 unnecessary or duplicated include files. 386 387 Always set a comment to determine the location of the following headers 388 before every group. 389 390 It is very important that '#include <fc_config.h>' is at the top of 391 every .c file (it need not be included from .h files). Some definitions in 392 fc_config.h will affect how the code is compiled, without which you can end 393 up with bad and untraceable memory bugs. 394 395 #ifdef HAVE_CONFIG_H 396 #include <fc_config.h> 397 #endif 398 399 #include <stdlib.h> 400 401 /* utility */ 402 #include "log.h" 403 404 /* common */ 405 #include "game.h" 406 407 #include "myfileheader.h" 408 409- For headers within a subdirectory path, the common rule is to set them 410 in an additional group, after the same group (don't forget the location 411 comment). 412 413 /* common */ 414 #include "game.h" 415 416 /* common/aicore */ 417 #include "pf_tools.h" 418 419 However, there is an exception to this. The last group is always the one 420 we are working on. So, if we are working on the common part, the order 421 should be: 422 423 /* common/aicore */ 424 #include "pf_tools.h" 425 426 /* common */ 427 #include "game.h" 428 429 Same observations with ai/ and server/. When working on the server/ 430 directory, the order should be: 431 432 /* ai */ 433 #include "aitools.h" 434 435 /* server */ 436 #include "srv_main.h" 437 438 and working on the ai/ directory: 439 440 /* server */ 441 #include "srv_main.h" 442 443 /* ai */ 444 #include "aitools.h" 445 446- Do not include headers in other headers if at all possible. Use forward 447 declarations for pointers to structs: 448 449 struct connection; 450 void a_function(struct connection *pconn); 451 452 instead of 453 454 #include "connection.h" 455 void a_function(struct connection *pconn); 456 457- Of course, never include headers of non-linked parts of the code. For 458 example, never include client/ headers into a server/ file. Also, in the 459 client/ directory, GUI specific headers are never included. Always, use 460 the common set of headers defined in client/include/. 461 462 463============================================================================ 464 Object-Oriented Programming 465============================================================================ 466Freeciv is not really object-oriented programming, but last written parts 467seems to tend to there. Also, there are more and more parts which are 468modular, so there are some observations to do: 469 470- Any function or member of a module must be prefixed by the name of this 471 module, or an abbreviation of it (but use the same prefix for all members 472 please!). Never set the module name as suffix! 473 474 /* Super mega cool module! */ 475 void smc_module_init(void); 476 void smc_module_free(void); 477 478 not 479 480 /* Super mega cool module! */ 481 void smc_module_init(void); 482 void sm_cool_free(void); 483 484 neither 485 486 /* Super mega cool module! */ 487 void init_smc_module(void); 488 void free_smc_module(void); 489 490- A function which allocates memory for a pointer variable should use the 491 suffix '_new'. The memory is freed by a corresponding function with the 492 suffix '_destroy'. 493 494 { 495 struct test *t = test_new(); 496 /* Do something. */ 497 test_destroy(t); 498 } 499 500- The suffix '_init' should be used for functions which initialize some 501 static data. The name of the corresponding function to deinitialize stuff 502 should use the suffix '_free' (see server/settings.c or common/map.c). 503 504 { 505 struct astring str; 506 507 astr_init(&str); 508 /* Do something. */ 509 astr_free(&str); 510 } 511 512 513============================================================================ 514 Miscellaneous 515============================================================================ 516 517- If an empty statement is needed, you should put an explanatory comment 518 in an empty block (i.e. {}): 519 520 while (*i++) { 521 /* Do nothing. */ 522 } 523 524- Use the postfix operator instead of the prefix operator when either will 525 work. That is, write "a++" instead of "++a". 526 527- Strive to make your code re-entrant (thread/recursion safe), as long as 528 this does not make the implementation too cumbersome or involved. 529 530- Strive to make your code modular: make it independent from other parts of 531 the codebase, and assume as little as possible about the circumstances in 532 which it is used. 533 534- Strive to avoid code duplication: if some part of the code is repeated in 535 several places, factor it out into a helper function. 536 537- Try to use static inline functions and const data instead of macros. 538 539- If helper functions internal to freeciv are added, prefix their names 540 with 'fc_'. Do not use 'my_' because it is also used by MySQL and could 541 be included in some libs. 542 543- Do not use assert() or die(); instead use the macros defined within 544 utility/log.h: 545 546 fc_assert(condition) 547 fc_assert_ret(condition) 548 fc_assert_ret_val(condition, val) 549 fc_assert_action(condition, action_on_failure) 550 fc_assert_exit(condition, action_on_failure) 551 552 fc_assert_msg(condition, message, ...) 553 fc_assert_ret_msg(condition, message, ...) 554 fc_assert_ret_val_msg(condition, val, message, ...) 555 fc_assert_action_msg(condition, action_on_failure, message, ...) 556 fc_assert_exit_msg(condition, action_on_failure, message, ...) 557 558 This way error conditions can be handled gracefully while still enforcing 559 invariants you expect not to be violated in the code. 560 (By default execution will continue with a warning, but it can be made 561 to halt by specifying the '-F' option to the client or server.) 562 563 int foo_get_id(const struct foo *pfoo) 564 { 565 fc_assert_ret_val(pfoo != NULL, -1); 566 return pfoo->id; 567 } 568 569- Do not put multiple conditions in the same fc_assert*() statement: 570 571 fc_assert(pfoo != NULL); 572 fc_assert(pfoo->id >= 0); 573 574 instead of 575 576 fc_assert(pfoo != NULL && pfoo->id >= 0); 577 578- Never include functionality also otherwise necessary inside fc_assert*(). 579 Such functionality would be missing from release builds where asserts 580 are disabled. If you want to assert return value of a critical function 581 call, make the call outside assert and place the return value to variable 582 and then assert value of that variable. 583 584- For strings containing multiple sentences, use a single space after periods 585 (not two, not zero, just one). 586 587- If you use a system specific feature, do not add #ifdef __CRAY__ or 588 something like that. Rather write a check for that feature for 589 configure.ac, and use a meaningful macro name in the source. 590 591- Always prototype global functions in the appropriate header file. Local 592 functions should always be declared as static. To catch these and some 593 other problems please use the following warning options "-Wall 594 -Wpointer-arith -Wcast-align -Wmissing-prototypes -Wmissing-declarations 595 -Wstrict-prototypes -Wnested-externs -Wl,--no-add-needed" if you use gcc. 596 597- Always check compilation with the configure option --enable-debug set. 598 599- Header files should be compatible with C++ but code (.c) files need not 600 be. This means some C++ keywords (like "this" or "class") may not be used 601 in header files. It also means casts on void pointers (which should be 602 avoided in C files) must be used in headers. 603 604- To assign null pointer, or to compare against one, use 'NULL' in C code, 605 and 'nullptr' in C++ code. 606 607- If you send patches, use "diff -u" (or "diff -r -u"). "git diff" works 608 correctly without extra parameters. 609 610 For further information, see: 611 <http://www.freeciv.org/wiki/How_to_Contribute>. 612 613 Also, name patch files descriptively (e.g. "fix-foo-bug-0.patch" is good, 614 but "freeciv.patch" is not). 615 616- When doing a "diff" for a patch, be sure to exclude unnecessary files by 617 using the "-X" argument to "diff". E.g.: 618 619 % diff -ruN -Xdiff_ignore freeciv_git freeciv >/tmp/fix-foo-bug-0.patch 620 621 A suggested "diff_ignore" file is included in the Freeciv distribution. 622 623============================================================================ 624