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