| 1 | Coding conventions in the Samba tree | 
|---|
| 2 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | 
|---|
| 3 |  | 
|---|
| 4 | .. contents:: | 
|---|
| 5 |  | 
|---|
| 6 | =========== | 
|---|
| 7 | Quick Start | 
|---|
| 8 | =========== | 
|---|
| 9 |  | 
|---|
| 10 | Coding style guidelines are about reducing the number of unnecessary | 
|---|
| 11 | reformatting patches and making things easier for developers to work | 
|---|
| 12 | together. | 
|---|
| 13 | You don't have to like them or even agree with them, but once put in place | 
|---|
| 14 | we all have to abide by them (or vote to change them).  However, coding | 
|---|
| 15 | style should never outweigh coding itself and so the guidelines | 
|---|
| 16 | described here are hopefully easy enough to follow as they are very | 
|---|
| 17 | common and supported by tools and editors. | 
|---|
| 18 |  | 
|---|
| 19 | The basic style, also mentioned in prog_guide4.txt, is the Linux kernel | 
|---|
| 20 | coding style (See Documentation/CodingStyle in the kernel source tree). This | 
|---|
| 21 | closely matches what most Samba developers use already anyways, with a few | 
|---|
| 22 | exceptions as mentioned below. | 
|---|
| 23 |  | 
|---|
| 24 | But to save you the trouble of reading the Linux kernel style guide, here | 
|---|
| 25 | are the highlights. | 
|---|
| 26 |  | 
|---|
| 27 | * Maximum Line Width is 80 Characters | 
|---|
| 28 | The reason is not about people with low-res screens but rather sticking | 
|---|
| 29 | to 80 columns prevents you from easily nesting more than one level of | 
|---|
| 30 | if statements or other code blocks.  Use source3/script/count_80_col.pl | 
|---|
| 31 | to check your changes. | 
|---|
| 32 |  | 
|---|
| 33 | * Use 8 Space Tabs to Indent | 
|---|
| 34 | No whitespace fillers. | 
|---|
| 35 |  | 
|---|
| 36 | * No Trailing Whitespace | 
|---|
| 37 | Use source3/script/strip_trail_ws.pl to clean up your files before | 
|---|
| 38 | committing. | 
|---|
| 39 |  | 
|---|
| 40 | * Follow the K&R guidelines.  We won't go through all of them here. Do you | 
|---|
| 41 | have a copy of "The C Programming Language" anyways right? You can also use | 
|---|
| 42 | the format_indent.sh script found in source3/script/ if all else fails. | 
|---|
| 43 |  | 
|---|
| 44 |  | 
|---|
| 45 |  | 
|---|
| 46 | ============ | 
|---|
| 47 | Editor Hints | 
|---|
| 48 | ============ | 
|---|
| 49 |  | 
|---|
| 50 | Emacs | 
|---|
| 51 | ----- | 
|---|
| 52 | Add the follow to your $HOME/.emacs file: | 
|---|
| 53 |  | 
|---|
| 54 | (add-hook 'c-mode-hook | 
|---|
| 55 | (lambda () | 
|---|
| 56 | (c-set-style "linux") | 
|---|
| 57 | (c-toggle-auto-state))) | 
|---|
| 58 |  | 
|---|
| 59 |  | 
|---|
| 60 | Vi | 
|---|
| 61 | -- | 
|---|
| 62 | (Thanks to SATOH Fumiyasu <fumiyas@osstech.jp> for these hints): | 
|---|
| 63 |  | 
|---|
| 64 | For the basic vi editor included with all variants of \*nix, add the | 
|---|
| 65 | following to $HOME/.exrc: | 
|---|
| 66 |  | 
|---|
| 67 | set tabstop=8 | 
|---|
| 68 | set shiftwidth=8 | 
|---|
| 69 |  | 
|---|
| 70 | For Vim, the following settings in $HOME/.vimrc will also deal with | 
|---|
| 71 | displaying trailing whitespace: | 
|---|
| 72 |  | 
|---|
| 73 | if has("syntax") && (&t_Co > 2 || has("gui_running")) | 
|---|
| 74 | syntax on | 
|---|
| 75 | function! ActivateInvisibleCharIndicator() | 
|---|
| 76 | syntax match TrailingSpace "[ \t]\+$" display containedin=ALL | 
|---|
| 77 | highlight TrailingSpace ctermbg=Red | 
|---|
| 78 | endf | 
|---|
| 79 | autocmd BufNewFile,BufRead * call ActivateInvisibleCharIndicator() | 
|---|
| 80 | endif | 
|---|
| 81 | " Show tabs, trailing whitespace, and continued lines visually | 
|---|
| 82 | set list listchars=tab:»·,trail:·,extends:⊠| 
|---|
| 83 |  | 
|---|
| 84 | " highlight overly long lines same as TODOs. | 
|---|
| 85 | set textwidth=80 | 
|---|
| 86 | autocmd BufNewFile,BufRead *.c,*.h exec 'match Todo /\%>' . &textwidth . 'v.\+/' | 
|---|
| 87 |  | 
|---|
| 88 |  | 
|---|
| 89 | ========================= | 
|---|
| 90 | FAQ & Statement Reference | 
|---|
| 91 | ========================= | 
|---|
| 92 |  | 
|---|
| 93 | Comments | 
|---|
| 94 | -------- | 
|---|
| 95 |  | 
|---|
| 96 | Comments should always use the standard C syntax.  C++ | 
|---|
| 97 | style comments are not currently allowed. | 
|---|
| 98 |  | 
|---|
| 99 | The lines before a comment should be empty. If the comment directly | 
|---|
| 100 | belongs to the following code, there should be no empty line | 
|---|
| 101 | after the comment, except if the comment contains a summary | 
|---|
| 102 | of multiple following code blocks. | 
|---|
| 103 |  | 
|---|
| 104 | This is good: | 
|---|
| 105 |  | 
|---|
| 106 | ... | 
|---|
| 107 | int i; | 
|---|
| 108 |  | 
|---|
| 109 | /* | 
|---|
| 110 | * This is a multi line comment, | 
|---|
| 111 | * which explains the logical steps we have to do: | 
|---|
| 112 | * | 
|---|
| 113 | * 1. We need to set i=5, because... | 
|---|
| 114 | * 2. We need to call complex_fn1 | 
|---|
| 115 | */ | 
|---|
| 116 |  | 
|---|
| 117 | /* This is a one line comment about i = 5. */ | 
|---|
| 118 | i = 5; | 
|---|
| 119 |  | 
|---|
| 120 | /* | 
|---|
| 121 | * This is a multi line comment, | 
|---|
| 122 | * explaining the call to complex_fn1() | 
|---|
| 123 | */ | 
|---|
| 124 | ret = complex_fn1(); | 
|---|
| 125 | if (ret != 0) { | 
|---|
| 126 | ... | 
|---|
| 127 |  | 
|---|
| 128 | /** | 
|---|
| 129 | * @brief This is a doxygen comment. | 
|---|
| 130 | * | 
|---|
| 131 | * This is a more detailed explanation of | 
|---|
| 132 | * this simple function. | 
|---|
| 133 | * | 
|---|
| 134 | * @param[in]   param1     The parameter value of the function. | 
|---|
| 135 | * | 
|---|
| 136 | * @param[out]  result1    The result value of the function. | 
|---|
| 137 | * | 
|---|
| 138 | * @return              0 on success and -1 on error. | 
|---|
| 139 | */ | 
|---|
| 140 | int example(int param1, int *result1); | 
|---|
| 141 |  | 
|---|
| 142 | This is bad: | 
|---|
| 143 |  | 
|---|
| 144 | ... | 
|---|
| 145 | int i; | 
|---|
| 146 | /* | 
|---|
| 147 | * This is a multi line comment, | 
|---|
| 148 | * which explains the logical steps we have to do: | 
|---|
| 149 | * | 
|---|
| 150 | * 1. We need to set i=5, because... | 
|---|
| 151 | * 2. We need to call complex_fn1 | 
|---|
| 152 | */ | 
|---|
| 153 | /* This is a one line comment about i = 5. */ | 
|---|
| 154 | i = 5; | 
|---|
| 155 | /* | 
|---|
| 156 | * This is a multi line comment, | 
|---|
| 157 | * explaining the call to complex_fn1() | 
|---|
| 158 | */ | 
|---|
| 159 | ret = complex_fn1(); | 
|---|
| 160 | if (ret != 0) { | 
|---|
| 161 | ... | 
|---|
| 162 |  | 
|---|
| 163 | /*This is a one line comment.*/ | 
|---|
| 164 |  | 
|---|
| 165 | /* This is a multi line comment, | 
|---|
| 166 | with some more words...*/ | 
|---|
| 167 |  | 
|---|
| 168 | /* | 
|---|
| 169 | * This is a multi line comment, | 
|---|
| 170 | * with some more words...*/ | 
|---|
| 171 |  | 
|---|
| 172 | Indention & Whitespace & 80 columns | 
|---|
| 173 | ----------------------------------- | 
|---|
| 174 |  | 
|---|
| 175 | To avoid confusion, indentations have to be tabs with length 8 (not 8 | 
|---|
| 176 | ' ' characters).  When wrapping parameters for function calls, | 
|---|
| 177 | align the parameter list with the first parameter on the previous line. | 
|---|
| 178 | Use tabs to get as close as possible and then fill in the final 7 | 
|---|
| 179 | characters or less with whitespace.  For example, | 
|---|
| 180 |  | 
|---|
| 181 | var1 = foo(arg1, arg2, | 
|---|
| 182 | arg3); | 
|---|
| 183 |  | 
|---|
| 184 | The previous example is intended to illustrate alignment of function | 
|---|
| 185 | parameters across lines and not as encourage for gratuitous line | 
|---|
| 186 | splitting.  Never split a line before columns 70 - 79 unless you | 
|---|
| 187 | have a really good reason.  Be smart about formatting. | 
|---|
| 188 |  | 
|---|
| 189 |  | 
|---|
| 190 | If, switch, & Code blocks | 
|---|
| 191 | ------------------------- | 
|---|
| 192 |  | 
|---|
| 193 | Always follow an 'if' keyword with a space but don't include additional | 
|---|
| 194 | spaces following or preceding the parentheses in the conditional. | 
|---|
| 195 | This is good: | 
|---|
| 196 |  | 
|---|
| 197 | if (x == 1) | 
|---|
| 198 |  | 
|---|
| 199 | This is bad: | 
|---|
| 200 |  | 
|---|
| 201 | if ( x == 1 ) | 
|---|
| 202 |  | 
|---|
| 203 | Yes we have a lot of code that uses the second form and we are trying | 
|---|
| 204 | to clean it up without being overly intrusive. | 
|---|
| 205 |  | 
|---|
| 206 | Note that this is a rule about parentheses following keywords and not | 
|---|
| 207 | functions.  Don't insert a space between the name and left parentheses when | 
|---|
| 208 | invoking functions. | 
|---|
| 209 |  | 
|---|
| 210 | Braces for code blocks used by for, if, switch, while, do..while, etc. | 
|---|
| 211 | should begin on the same line as the statement keyword and end on a line | 
|---|
| 212 | of their own. You should always include braces, even if the block only | 
|---|
| 213 | contains one statement.  NOTE: Functions are different and the beginning left | 
|---|
| 214 | brace should be located in the first column on the next line. | 
|---|
| 215 |  | 
|---|
| 216 | If the beginning statement has to be broken across lines due to length, | 
|---|
| 217 | the beginning brace should be on a line of its own. | 
|---|
| 218 |  | 
|---|
| 219 | The exception to the ending rule is when the closing brace is followed by | 
|---|
| 220 | another language keyword such as else or the closing while in a do..while | 
|---|
| 221 | loop. | 
|---|
| 222 |  | 
|---|
| 223 | Good examples: | 
|---|
| 224 |  | 
|---|
| 225 | if (x == 1) { | 
|---|
| 226 | printf("good\n"); | 
|---|
| 227 | } | 
|---|
| 228 |  | 
|---|
| 229 | for (x=1; x<10; x++) { | 
|---|
| 230 | print("%d\n", x); | 
|---|
| 231 | } | 
|---|
| 232 |  | 
|---|
| 233 | for (really_really_really_really_long_var_name=0; | 
|---|
| 234 | really_really_really_really_long_var_name<10; | 
|---|
| 235 | really_really_really_really_long_var_name++) | 
|---|
| 236 | { | 
|---|
| 237 | print("%d\n", really_really_really_really_long_var_name); | 
|---|
| 238 | } | 
|---|
| 239 |  | 
|---|
| 240 | do { | 
|---|
| 241 | printf("also good\n"); | 
|---|
| 242 | } while (1); | 
|---|
| 243 |  | 
|---|
| 244 | Bad examples: | 
|---|
| 245 |  | 
|---|
| 246 | while (1) | 
|---|
| 247 | { | 
|---|
| 248 | print("I'm in a loop!\n"); } | 
|---|
| 249 |  | 
|---|
| 250 | for (x=1; | 
|---|
| 251 | x<10; | 
|---|
| 252 | x++) | 
|---|
| 253 | { | 
|---|
| 254 | print("no good\n"); | 
|---|
| 255 | } | 
|---|
| 256 |  | 
|---|
| 257 | if (i < 10) | 
|---|
| 258 | print("I should be in braces.\n"); | 
|---|
| 259 |  | 
|---|
| 260 |  | 
|---|
| 261 | Goto | 
|---|
| 262 | ---- | 
|---|
| 263 |  | 
|---|
| 264 | While many people have been academically taught that "goto"s are | 
|---|
| 265 | fundamentally evil, they can greatly enhance readability and reduce memory | 
|---|
| 266 | leaks when used as the single exit point from a function. But in no Samba | 
|---|
| 267 | world what so ever is a goto outside of a function or block of code a good | 
|---|
| 268 | idea. | 
|---|
| 269 |  | 
|---|
| 270 | Good Examples: | 
|---|
| 271 |  | 
|---|
| 272 | int function foo(int y) | 
|---|
| 273 | { | 
|---|
| 274 | int *z = NULL; | 
|---|
| 275 | int ret = 0; | 
|---|
| 276 |  | 
|---|
| 277 | if (y < 10) { | 
|---|
| 278 | z = malloc(sizeof(int)*y); | 
|---|
| 279 | if (!z) { | 
|---|
| 280 | ret = 1; | 
|---|
| 281 | goto done; | 
|---|
| 282 | } | 
|---|
| 283 | } | 
|---|
| 284 |  | 
|---|
| 285 | print("Allocated %d elements.\n", y); | 
|---|
| 286 |  | 
|---|
| 287 | done: | 
|---|
| 288 | if (z) { | 
|---|
| 289 | free(z); | 
|---|
| 290 | } | 
|---|
| 291 |  | 
|---|
| 292 | return ret; | 
|---|
| 293 | } | 
|---|
| 294 |  | 
|---|
| 295 |  | 
|---|
| 296 | Checking Pointer Values | 
|---|
| 297 | ----------------------- | 
|---|
| 298 |  | 
|---|
| 299 | When invoking functions that return pointer values, either of the following | 
|---|
| 300 | are acceptable. Use your best judgement and choose the more readable option. | 
|---|
| 301 | Remember that many other persons will review it: | 
|---|
| 302 |  | 
|---|
| 303 | if ((x = malloc(sizeof(short)*10)) == NULL ) { | 
|---|
| 304 | fprintf(stderr, "Unable to alloc memory!\n"); | 
|---|
| 305 | } | 
|---|
| 306 |  | 
|---|
| 307 | or: | 
|---|
| 308 |  | 
|---|
| 309 | x = malloc(sizeof(short)*10); | 
|---|
| 310 | if (!x) { | 
|---|
| 311 | fprintf(stderr, "Unable to alloc memory!\n"); | 
|---|
| 312 | } | 
|---|
| 313 |  | 
|---|
| 314 |  | 
|---|
| 315 | Primitive Data Types | 
|---|
| 316 | -------------------- | 
|---|
| 317 |  | 
|---|
| 318 | Samba has large amounts of historical code which makes use of data types | 
|---|
| 319 | commonly supported by the C99 standard. However, at the time such types | 
|---|
| 320 | as boolean and exact width integers did not exist and Samba developers | 
|---|
| 321 | were forced to provide their own.  Now that these types are guaranteed to | 
|---|
| 322 | be available either as part of the compiler C99 support or from | 
|---|
| 323 | lib/replace/, new code should adhere to the following conventions: | 
|---|
| 324 |  | 
|---|
| 325 | * Booleans are of type "bool" (not BOOL) | 
|---|
| 326 | * Boolean values are "true" and "false" (not True or False) | 
|---|
| 327 | * Exact width integers are of type [u]int[8|16|32|64]_t | 
|---|
| 328 |  | 
|---|
| 329 |  | 
|---|
| 330 | Typedefs | 
|---|
| 331 | -------- | 
|---|
| 332 |  | 
|---|
| 333 | Samba tries to avoid "typedef struct { .. } x_t;" so we do always try to use | 
|---|
| 334 | "struct x { .. };". We know there are still such typedefs in the code, | 
|---|
| 335 | but for new code, please don't do that anymore. | 
|---|
| 336 |  | 
|---|
| 337 | Make use of helper variables | 
|---|
| 338 | ---------------------------- | 
|---|
| 339 |  | 
|---|
| 340 | Please try to avoid passing function calls as function parameters | 
|---|
| 341 | in new code. This makes the code much easier to read and | 
|---|
| 342 | it's also easier to use the "step" command within gdb. | 
|---|
| 343 |  | 
|---|
| 344 | Good Example: | 
|---|
| 345 |  | 
|---|
| 346 | char *name; | 
|---|
| 347 |  | 
|---|
| 348 | name = get_some_name(); | 
|---|
| 349 | if (name == NULL) { | 
|---|
| 350 | ... | 
|---|
| 351 | } | 
|---|
| 352 |  | 
|---|
| 353 | ret = some_function_my_name(name); | 
|---|
| 354 | ... | 
|---|
| 355 |  | 
|---|
| 356 |  | 
|---|
| 357 | Bad Example: | 
|---|
| 358 |  | 
|---|
| 359 | ret = some_function_my_name(get_some_name()); | 
|---|
| 360 | ... | 
|---|
| 361 |  | 
|---|