1Coding conventions in the Samba tree 2~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 3 4.. contents:: 5 6=========== 7Quick Start 8=========== 9 10Coding style guidelines are about reducing the number of unnecessary 11reformatting patches and making things easier for developers to work 12together. 13You don't have to like them or even agree with them, but once put in place 14we all have to abide by them (or vote to change them). However, coding 15style should never outweigh coding itself and so the guidelines 16described here are hopefully easy enough to follow as they are very 17common and supported by tools and editors. 18 19The basic style for C code is the Linux kernel coding style (See 20Documentation/CodingStyle in the kernel source tree). This closely matches 21what most Samba developers use already anyways, with a few exceptions as 22mentioned below. 23 24The coding style for Python code is documented in PEP8, 25https://www.python.org/dev/peps/pep-0008/. New Python code should be compatible 26with Python 2.6, 2.7, and Python 3.4 onwards. This means using Python 3 syntax 27with the appropriate 'from __future__' imports. 28 29But to save you the trouble of reading the Linux kernel style guide, here 30are the highlights. 31 32* Maximum Line Width is 80 Characters 33 The reason is not about people with low-res screens but rather sticking 34 to 80 columns prevents you from easily nesting more than one level of 35 if statements or other code blocks. Use source3/script/count_80_col.pl 36 to check your changes. 37 38* Use 8 Space Tabs to Indent 39 No whitespace fillers. 40 41* No Trailing Whitespace 42 Use source3/script/strip_trail_ws.pl to clean up your files before 43 committing. 44 45* Follow the K&R guidelines. We won't go through all of them here. Do you 46 have a copy of "The C Programming Language" anyways right? You can also use 47 the format_indent.sh script found in source3/script/ if all else fails. 48 49 50 51============ 52Editor Hints 53============ 54 55Emacs 56----- 57Add the follow to your $HOME/.emacs file: 58 59 (add-hook 'c-mode-hook 60 (lambda () 61 (c-set-style "linux") 62 (c-toggle-auto-state))) 63 64 65Vi 66-- 67(Thanks to SATOH Fumiyasu <fumiyas@osstech.jp> for these hints): 68 69For the basic vi editor included with all variants of \*nix, add the 70following to $HOME/.exrc: 71 72 set tabstop=8 73 set shiftwidth=8 74 75For Vim, the following settings in $HOME/.vimrc will also deal with 76displaying trailing whitespace: 77 78 if has("syntax") && (&t_Co > 2 || has("gui_running")) 79 syntax on 80 function! ActivateInvisibleCharIndicator() 81 syntax match TrailingSpace "[ \t]\+$" display containedin=ALL 82 highlight TrailingSpace ctermbg=Red 83 endf 84 autocmd BufNewFile,BufRead * call ActivateInvisibleCharIndicator() 85 endif 86 " Show tabs, trailing whitespace, and continued lines visually 87 set list listchars=tab:»·,trail:·,extends:… 88 89 " highlight overly long lines same as TODOs. 90 set textwidth=80 91 autocmd BufNewFile,BufRead *.c,*.h exec 'match Todo /\%>' . &textwidth . 'v.\+/' 92 93clang-format 94------------ 95BasedOnStyle: LLVM 96IndentWidth: 8 97UseTab: true 98BreakBeforeBraces: Linux 99AllowShortIfStatementsOnASingleLine: false 100IndentCaseLabels: false 101BinPackParameters: false 102BinPackArguments: false 103SortIncludes: false 104 105 106========================= 107FAQ & Statement Reference 108========================= 109 110Comments 111-------- 112 113Comments should always use the standard C syntax. C++ 114style comments are not currently allowed. 115 116The lines before a comment should be empty. If the comment directly 117belongs to the following code, there should be no empty line 118after the comment, except if the comment contains a summary 119of multiple following code blocks. 120 121This is good: 122 123 ... 124 int i; 125 126 /* 127 * This is a multi line comment, 128 * which explains the logical steps we have to do: 129 * 130 * 1. We need to set i=5, because... 131 * 2. We need to call complex_fn1 132 */ 133 134 /* This is a one line comment about i = 5. */ 135 i = 5; 136 137 /* 138 * This is a multi line comment, 139 * explaining the call to complex_fn1() 140 */ 141 ret = complex_fn1(); 142 if (ret != 0) { 143 ... 144 145 /** 146 * @brief This is a doxygen comment. 147 * 148 * This is a more detailed explanation of 149 * this simple function. 150 * 151 * @param[in] param1 The parameter value of the function. 152 * 153 * @param[out] result1 The result value of the function. 154 * 155 * @return 0 on success and -1 on error. 156 */ 157 int example(int param1, int *result1); 158 159This is bad: 160 161 ... 162 int i; 163 /* 164 * This is a multi line comment, 165 * which explains the logical steps we have to do: 166 * 167 * 1. We need to set i=5, because... 168 * 2. We need to call complex_fn1 169 */ 170 /* This is a one line comment about i = 5. */ 171 i = 5; 172 /* 173 * This is a multi line comment, 174 * explaining the call to complex_fn1() 175 */ 176 ret = complex_fn1(); 177 if (ret != 0) { 178 ... 179 180 /*This is a one line comment.*/ 181 182 /* This is a multi line comment, 183 with some more words...*/ 184 185 /* 186 * This is a multi line comment, 187 * with some more words...*/ 188 189Indention & Whitespace & 80 columns 190----------------------------------- 191 192To avoid confusion, indentations have to be tabs with length 8 (not 8 193' ' characters). When wrapping parameters for function calls, 194align the parameter list with the first parameter on the previous line. 195Use tabs to get as close as possible and then fill in the final 7 196characters or less with whitespace. For example, 197 198 var1 = foo(arg1, arg2, 199 arg3); 200 201The previous example is intended to illustrate alignment of function 202parameters across lines and not as encourage for gratuitous line 203splitting. Never split a line before columns 70 - 79 unless you 204have a really good reason. Be smart about formatting. 205 206One exception to the previous rule is function calls, declarations, and 207definitions. In function calls, declarations, and definitions, either the 208declaration is a one-liner, or each parameter is listed on its own 209line. The rationale is that if there are many parameters, each one 210should be on its own line to make tracking interface changes easier. 211 212 213If, switch, & Code blocks 214------------------------- 215 216Always follow an 'if' keyword with a space but don't include additional 217spaces following or preceding the parentheses in the conditional. 218This is good: 219 220 if (x == 1) 221 222This is bad: 223 224 if ( x == 1 ) 225 226Yes we have a lot of code that uses the second form and we are trying 227to clean it up without being overly intrusive. 228 229Note that this is a rule about parentheses following keywords and not 230functions. Don't insert a space between the name and left parentheses when 231invoking functions. 232 233Braces for code blocks used by for, if, switch, while, do..while, etc. 234should begin on the same line as the statement keyword and end on a line 235of their own. You should always include braces, even if the block only 236contains one statement. NOTE: Functions are different and the beginning left 237brace should be located in the first column on the next line. 238 239If the beginning statement has to be broken across lines due to length, 240the beginning brace should be on a line of its own. 241 242The exception to the ending rule is when the closing brace is followed by 243another language keyword such as else or the closing while in a do..while 244loop. 245 246Good examples: 247 248 if (x == 1) { 249 printf("good\n"); 250 } 251 252 for (x=1; x<10; x++) { 253 print("%d\n", x); 254 } 255 256 for (really_really_really_really_long_var_name=0; 257 really_really_really_really_long_var_name<10; 258 really_really_really_really_long_var_name++) 259 { 260 print("%d\n", really_really_really_really_long_var_name); 261 } 262 263 do { 264 printf("also good\n"); 265 } while (1); 266 267Bad examples: 268 269 while (1) 270 { 271 print("I'm in a loop!\n"); } 272 273 for (x=1; 274 x<10; 275 x++) 276 { 277 print("no good\n"); 278 } 279 280 if (i < 10) 281 print("I should be in braces.\n"); 282 283 284Goto 285---- 286 287While many people have been academically taught that "goto"s are 288fundamentally evil, they can greatly enhance readability and reduce memory 289leaks when used as the single exit point from a function. But in no Samba 290world what so ever is a goto outside of a function or block of code a good 291idea. 292 293Good Examples: 294 295 int function foo(int y) 296 { 297 int *z = NULL; 298 int ret = 0; 299 300 if (y < 10) { 301 z = malloc(sizeof(int) * y); 302 if (z == NULL) { 303 ret = 1; 304 goto done; 305 } 306 } 307 308 print("Allocated %d elements.\n", y); 309 310 done: 311 if (z != NULL) { 312 free(z); 313 } 314 315 return ret; 316 } 317 318 319Primitive Data Types 320-------------------- 321 322Samba has large amounts of historical code which makes use of data types 323commonly supported by the C99 standard. However, at the time such types 324as boolean and exact width integers did not exist and Samba developers 325were forced to provide their own. Now that these types are guaranteed to 326be available either as part of the compiler C99 support or from 327lib/replace/, new code should adhere to the following conventions: 328 329 * Booleans are of type "bool" (not BOOL) 330 * Boolean values are "true" and "false" (not True or False) 331 * Exact width integers are of type [u]int[8|16|32|64]_t 332 333Most of the time a good name for a boolean variable is 'ok'. Here is an 334example we often use: 335 336 bool ok; 337 338 ok = foo(); 339 if (!ok) { 340 /* do something */ 341 } 342 343It makes the code more readable and is easy to debug. 344 345Typedefs 346-------- 347 348Samba tries to avoid "typedef struct { .. } x_t;" so we do always try to use 349"struct x { .. };". We know there are still such typedefs in the code, 350but for new code, please don't do that anymore. 351 352Initialize pointers 353------------------- 354 355All pointer variables MUST be initialized to NULL. History has 356demonstrated that uninitialized pointer variables have lead to various 357bugs and security issues. 358 359Pointers MUST be initialized even if the assignment directly follows 360the declaration, like pointer2 in the example below, because the 361instructions sequence may change over time. 362 363Good Example: 364 365 char *pointer1 = NULL; 366 char *pointer2 = NULL; 367 368 pointer2 = some_func2(); 369 370 ... 371 372 pointer1 = some_func1(); 373 374Bad Example: 375 376 char *pointer1; 377 char *pointer2; 378 379 pointer2 = some_func2(); 380 381 ... 382 383 pointer1 = some_func1(); 384 385Make use of helper variables 386---------------------------- 387 388Please try to avoid passing function calls as function parameters 389in new code. This makes the code much easier to read and 390it's also easier to use the "step" command within gdb. 391 392Good Example: 393 394 char *name = NULL; 395 int ret; 396 397 name = get_some_name(); 398 if (name == NULL) { 399 ... 400 } 401 402 ret = some_function_my_name(name); 403 ... 404 405 406Bad Example: 407 408 ret = some_function_my_name(get_some_name()); 409 ... 410 411Please try to avoid passing function return values to if- or 412while-conditions. The reason for this is better handling of code under a 413debugger. 414 415Good example: 416 417 x = malloc(sizeof(short)*10); 418 if (x == NULL) { 419 fprintf(stderr, "Unable to alloc memory!\n"); 420 } 421 422Bad example: 423 424 if ((x = malloc(sizeof(short)*10)) == NULL ) { 425 fprintf(stderr, "Unable to alloc memory!\n"); 426 } 427 428There are exceptions to this rule. One example is walking a data structure in 429an iterator style: 430 431 while ((opt = poptGetNextOpt(pc)) != -1) { 432 ... do something with opt ... 433 } 434 435Another exception: DBG messages for example printing a SID or a GUID: 436Here we don't expect any surprise from the printing functions, and the 437main reason of this guideline is to make debugging easier. That reason 438rarely exists for this particular use case, and we gain some 439efficiency because the DBG_ macros don't evaluate their arguments if 440the debuglevel is not high enough. 441 442 if (!NT_STATUS_IS_OK(status)) { 443 struct dom_sid_buf sid_buf; 444 struct GUID_txt_buf guid_buf; 445 DBG_WARNING( 446 "objectSID [%s] for GUID [%s] invalid\n", 447 dom_sid_str_buf(objectsid, &sid_buf), 448 GUID_buf_string(&cache->entries[idx], &guid_buf)); 449 } 450 451But in general, please try to avoid this pattern. 452 453 454Control-Flow changing macros 455---------------------------- 456 457Macros like NT_STATUS_NOT_OK_RETURN that change control flow 458(return/goto/etc) from within the macro are considered bad, because 459they look like function calls that never change control flow. Please 460do not use them in new code. 461 462The only exception is the test code that depends repeated use of calls 463like CHECK_STATUS, CHECK_VAL and others. 464 465 466Error and out logic 467------------------- 468 469Don't do this: 470 471 frame = talloc_stackframe(); 472 473 if (ret == LDB_SUCCESS) { 474 if (result->count == 0) { 475 ret = LDB_ERR_NO_SUCH_OBJECT; 476 } else { 477 struct ldb_message *match = 478 get_best_match(dn, result); 479 if (match == NULL) { 480 TALLOC_FREE(frame); 481 return LDB_ERR_OPERATIONS_ERROR; 482 } 483 *msg = talloc_move(mem_ctx, &match); 484 } 485 } 486 487 TALLOC_FREE(frame); 488 return ret; 489 490It should be: 491 492 frame = talloc_stackframe(); 493 494 if (ret != LDB_SUCCESS) { 495 TALLOC_FREE(frame); 496 return ret; 497 } 498 499 if (result->count == 0) { 500 TALLOC_FREE(frame); 501 return LDB_ERR_NO_SUCH_OBJECT; 502 } 503 504 match = get_best_match(dn, result); 505 if (match == NULL) { 506 TALLOC_FREE(frame); 507 return LDB_ERR_OPERATIONS_ERROR; 508 } 509 510 *msg = talloc_move(mem_ctx, &match); 511 TALLOC_FREE(frame); 512 return LDB_SUCCESS; 513 514 515DEBUG statements 516---------------- 517 518Use these following macros instead of DEBUG: 519 520DBG_ERR log level 0 error conditions 521DBG_WARNING log level 1 warning conditions 522DBG_NOTICE log level 3 normal, but significant, condition 523DBG_INFO log level 5 informational message 524DBG_DEBUG log level 10 debug-level message 525 526Example usage: 527 528DBG_ERR("Memory allocation failed\n"); 529DBG_DEBUG("Received %d bytes\n", count); 530 531The messages from these macros are automatically prefixed with the 532function name. 533