| 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 | 
 | 
|---|