update guidelines to explain indent parameteres (bug #4742)
[asterisk/asterisk.git] / doc / CODING-GUIDELINES
1 == Asterisk Patch/Coding Guidelines ==
2
3 To be accepted into the codebase, all non-trivial changes must be
4 disclaimed to Digium or placed in the public domain. For more information
5 see http://bugs.digium.com
6
7 Patches should be in the form of a unified (-u) diff, made from the directory
8 above the top-level Asterisk source directory. For example:
9
10 - the base code you are working from is in ~/work/asterisk-base
11 - the changes are in ~/work/asterisk-new
12
13 ~/work$ diff -urN asterisk-base asterisk-new
14
15 All code, filenames, function names and comments must be in ENGLISH.
16
17 Do not declare variables mid-function (e.g. like recent GNU compilers support)
18 since it is harder to read and not portable to GCC 2.95 and others.
19
20 Don't annotate your changes with comments like "/* JMG 4/20/04 */";
21 Comments should explain what the code does, not when something was changed
22 or who changed it.
23
24 Don't make unnecessary whitespace changes throughout the code.
25
26 Don't use C++ type (//) comments.
27
28 Try to match the existing formatting of the file you are working on.
29
30 Functions and variables that are not intended to be used outside the module
31 must be declared static.
32
33 When reading integer numeric input with scanf (or variants), do _NOT_ use '%i'
34 unless you specifically want to allow non-base-10 input; '%d' is always a better
35 choice, since it will not silently turn numbers with leading zeros into base-8.
36
37 Use spaces instead of tabs when aligning in-line comments or #defines (this makes 
38 your comments aligned even if the code is viewed with another tabsize)
39
40 Roughly, Asterisk code formatting guidelines are generally equivalent to the 
41 following:
42
43 # indent -i4 -ts4 -br -brs -cdw -cli0 -ce -nbfda -npcs -nprs -npsl -saf -sai -saw foo.c
44
45 this means in verbose:
46  -i4:    indent level 4
47  -ts4:   tab size 4
48  -br:    braces on if line
49  -brs:   braces on struct decl line
50  -cdw:   cuddle do while
51  -cli0:  case indentation 0
52  -ce:    cuddle else
53  -nbfda: dont break function decl args
54  -npcs:  no space after function call names
55  -nprs:  no space after parentheses
56  -npsl:  dont break procedure type
57  -saf:   space after for
58  -sai:   space after if
59  -saw:   space after while
60
61 Function calls and arguments should be spaced in a consistent way across
62 the codebase.
63 GOOD: foo(arg1, arg2);
64 GOOD: foo(arg1,arg2);   /* Acceptable but not preferred */
65 BAD: foo (arg1, arg2);
66 BAD: foo( arg1, arg2 );
67 BAD: foo(arg1, arg2,arg3);
68
69 Don't treat keywords (if, while, do, return) as if they were functions;
70 leave space between the keyword and the expression used (if any). For 'return',
71 don't even put parentheses around the expression, since they are not
72 required.
73
74 There is no shortage of whitespace characters :-) Use them when they make
75 the code easier to read. For example:
76
77 for (str=foo;str;str=str->next)
78
79 is harder to read than
80
81 for (str = foo; str; str = str->next)
82
83 Following are examples of how code should be formatted.
84
85 Functions:
86 int foo(int a, char *s)
87 {
88         return 0;
89 }
90
91 If statements:
92 if (foo) {
93         bar();
94 } else {
95         blah();
96 }
97
98 Case statements:
99 switch (foo) {
100 case BAR:
101         blah();
102         break;
103 case OTHER:
104         other();
105         break;
106 }
107
108 No nested statements without braces, e.g.:
109
110 for (x = 0; x < 5; x++)
111         if (foo) 
112                 if (bar)
113                         baz();
114
115 instead do:
116 for (x = 0; x < 5; x++) {
117         if (foo) {
118                 if (bar)
119                         baz();
120         }
121 }
122
123 Don't build code like this:
124
125 if (foo) {
126         /* .... 50 lines of code ... */
127 } else {
128         result = 0;
129         return;
130 }
131
132 Instead, try to minimize the number of lines of code that need to be
133 indented, by only indenting the shortest case of the 'if'
134 statement, like so:
135
136 if !(foo) {
137         result = 0;
138         return;
139 }
140
141 .... 50 lines of code ....
142
143 When this technique is used properly, it makes functions much easier to read
144 and follow, especially those with more than one or two 'setup' operations
145 that must succeed for the rest of the function to be able to execute.
146
147 Proper use of this technique may occasionally result in the need for a
148 label/goto combination so that error/failure conditions can exit the
149 function while still performing proper cleanup. This is not a bad thing!
150 Use of goto in this situation is encouraged, since it removes the need
151 for excess code indenting without requiring duplication of cleanup code.
152        
153 Make sure you never use an uninitialized variable.  The compiler will 
154 usually warn you if you do so. However, do not go too far the other way,
155 and needlessly initialize variables that do not require it. If the first
156 time you use a variable in a function is to store a value there, then
157 initializing it at declaration is pointless, and will generate extra
158 object code and data in the resulting binary with no purpose. When in doubt,
159 trust the compiler to tell you when you need to initialize a variable;
160 if it does not warn you, initialization is not needed.
161
162 Do not explicitly cast 'void *' into any other type, nor should you cast any
163 other type into 'void *'. Implicit casts to/from 'void *' are explicitly
164 allowed by the C specification. This means the results of malloc(), calloc(),
165 alloca(), and similar functions do not _ever_ need to be cast to a specific
166 type, and when you are passing a pointer to (for example) a callback function
167 that accepts a 'void *' you do not need to cast into that type.
168
169 Name global variables (or local variables when you have a lot of them or
170 are in a long function) something that will make sense to aliens who
171 find your code in 100 years.  All variable names should be in lower 
172 case, except when following external APIs or specifications that normally
173 use upper- or mixed-case variable names; in that situation, it is
174 preferable to follow the external API/specification for ease of
175 understanding.
176
177 Make some indication in the name of global variables which represent
178 options that they are in fact intended to be global.
179  e.g.: static char global_something[80]
180
181 Don't use 'typedef' just to shorten the amount of typing; there is no substantial
182 benefit in this:
183
184 struct foo {
185         int bar;
186 };
187 typedef foo_t struct foo;
188
189 In fact, don't use 'variable type' suffixes at all; it's much preferable to
190 just type 'struct foo' rather than 'foo_s'.
191
192 Use enums rather than long lists of #define-d numeric constants when possible;
193 this allows structure members, local variables and function arguments to
194 be declared as using the enum's type. For example:
195
196 enum option {
197   OPT_FOO = 1
198   OPT_BAR = 2
199   OPT_BAZ = 4
200 };
201
202 static enum option global_option;
203
204 static handle_option(const enum option opt)
205 {
206   ...
207 }
208
209 Note: The compiler will _not_ force you to pass an entry from the enum
210 as an argument to this function; this recommendation serves only to make
211 the code clearer and somewhat self-documenting.
212
213 When making applications, always ast_strdupa(data) to a local pointer if
214 you intend to parse the incoming data string.
215
216         if (data)
217                 mydata = ast_strdupa(data);
218
219 Use ast_separate_app_args() to separate the arguments to your application
220 once you have made a local copy of the string.
221
222 Use strsep() for parsing strings when possible; there is no worry about
223 're-entrancy' as with strtok(), and even though it modifies the original
224 string (which the man page warns about), in many cases that is exactly
225 what you want!
226
227 Always derefrence or localize pointers to things that are not yours like
228 channel members in a channel that is not associated with the current 
229 thread and for which you do not have a lock.
230         channame = ast_strdupa(otherchan->name);
231
232 If you do the same or a similar operation more than 1 time, make it a
233 function or macro.
234
235 Make sure you are not duplicating any functionality already found in an
236 API call somewhere.  If you are duplicating functionality found in 
237 another static function, consider the value of creating a new API call 
238 which can be shared.
239
240 As a common example of this point, make an effort to use the lockable
241 linked-list macros found in include/asterisk/linkedlists.h. They are
242 efficient, easy to use and provide every operation that should be
243 necessary for managing a singly-linked list (if something is missing,
244 let us know!). Just because you see other open-coded list implementations
245 in the source tree is no reason to continue making new copies of
246 that code... There are also a number of common string manipulation
247 and timeval manipulation functions in asterisk/strings.h and asterisk/time.h;
248 use them when possible.
249
250 When you achieve your desired functionalty, make another few refactor
251 passes over the code to optimize it.
252
253 Before submitting a patch, *read* the actual patch file to be sure that 
254 all the changes you expect to be there are, and that there are no 
255 surprising changes you did not expect.
256
257 If you are asked to make changes to your patch, there is a good chance
258 the changes will introduce bugs, check it even more at this stage.
259
260 Avoid needless malloc(), strdup() calls. If you only need the value in
261 the scope of your function try ast_strdupa() or declare structs on the
262 stack and pass a pointer to them. However, be careful to _never_ call
263 alloca(), ast_strdupa() or similar functions in the argument list
264 of a function you are calling; this can cause very strange stack
265 arrangements and produce unexpected behavior.
266
267 If you are going to reuse a computed value, save it in a variable
268 instead of recomputing it over and over.  This can prevent you from 
269 making a mistake in subsequent computations, make it easier to correct
270 if the formula has an error and may or may not help optimization but 
271 will at least help readability.
272
273 Just an example, so don't over analyze it, that'd be a shame:
274
275 const char *prefix = "pre";     
276 const char *postfix = "post";
277 char *newname;
278 char *name = "data";
279
280 if (name && (newname = alloca(strlen(name) + strlen(prefix) + strlen(postfix) + 3)))
281         snprintf(newname, strlen(name) + strlen(prefix) + strlen(postfix) + 3, "%s/%s/%s", prefix, name, postfix);
282
283 vs
284
285 const char *prefix = "pre";
286 const char *postfix = "post";
287 char *newname;
288 char *name = "data";
289 int len = 0;
290
291 if (name && (len = strlen(name) + strlen(prefix) + strlen(postfix) + 3) && (newname = alloca(len)))
292         snprintf(newname, len, "%s/%s/%s", prefix, name, postfix);
293
294
295 Use const on pointer arguments which your function will not be modifying, as this 
296 allows the compiler to make certain optimizations. In general, use 'const'
297 on any argument that you have no direct intention of modifying, as it can
298 catch logic/typing errors in your code when you use the argument variable
299 in a way that you did not intend.
300
301 Don't use strncpy for copying whole strings; it does not guarantee that the
302 output buffer will be null-terminated. Use ast_copy_string instead, which
303 is also slightly more efficient (and allows passing the actual buffer
304 size, which makes the code clearer).
305
306 Don't use ast_copy_string (or any length-limited copy function) for copying
307 fixed (known at compile time) strings into buffers, if the buffer is something
308 that has been allocated in the function doing the copying. In that case, you
309 know at the time you are writing the code whether the buffer is large enough
310 for the fixed string or not, and if it's not, your code won't work anyway!
311 Use strcpy() for this operation, or directly set the first two characters
312 of the buffer if you are just trying store a one-character string in the
313 buffer. If you are trying to 'empty' the buffer, just store a single
314 NULL character ('\0') in the first byte of the buffer; nothing else is
315 needed, and any other method is wasteful.
316
317 In addition, if the previous operations in the function have already
318 determined that the buffer in use is adequately sized to hold the string
319 you wish to put into it (even if you did not allocate the buffer yourself),
320 use a direct strcpy(), as it can be inlined and optimized to simple
321 processor operations, unlike ast_copy_string().
322
323 When allocating/zeroing memory for a structure, try to use code like this:
324
325 struct foo *tmp;
326
327 ...
328
329 tmp = malloc(sizeof(*tmp));
330 if (tmp)
331         memset(tmp, 0, sizeof(*tmp));
332
333 This eliminates duplication of the 'struct foo' identifier, which makes the
334 code easier to read and also ensures that if it is copy-and-pasted it won't
335 require as much editing. In fact, you can even use:
336
337 struct foo *tmp;
338
339 ...
340
341 tmp = calloc(1, sizeof(*tmp));
342
343 This will allocate and zero the memory in a single operation.
344
345 == CLI Commands ==
346
347 New CLI commands should be named using the module's name, followed by a verb
348 and then any parameters that the command needs. For example:
349
350 *CLI> iax2 show peer <peername>
351
352 not
353
354 *CLI> show iax2 peer <peername>
355
356 == New dialplan applications/functions ==
357
358 There are two methods of adding functionality to the Asterisk
359 dialplan: applications and functions. Applications (found generally in
360 the apps/ directory) should be collections of code that interact with
361 a channel and/or user in some significant way. Functions (which can be
362 provided by any type of module) are used when the provided
363 functionality is simple... getting/retrieving a value, for
364 example. Functions should also be used when the operation is in no way
365 related to a channel (a computation or string operation, for example).
366
367 Applications are registered and invoked using the
368 ast_register_application function; see the apps/app_skel.c file for an
369 example.
370
371 Functions are registered using 'struct ast_custom_function'
372 structures and the ast_custom_function_register function.
373
374 == Doxygen API Documentation Guidelines ==
375
376 When writing Asterisk API documentation the following format should be
377 followed.
378
379 /*!
380  * \brief Do interesting stuff.
381  * \param thing1 interesting parameter 1.
382  * \param thing2 interesting parameter 2.
383  *
384  * This function does some interesting stuff.
385  *
386  * \return zero on success, -1 on error.
387  */
388 int ast_interesting_stuff(int thing1, int thing2)
389 {
390         return 0;
391 }
392
393 Notice the use of the \param, \brief, and \return constructs.  These should be
394 used to describe the corresponding pieces of the function being documented.
395 Also notice the blank line after the last \param directive.  All doxygen
396 comments must be in one /*! */ block.  If the function or struct does not need
397 an extended description it can be left out.
398
399 Please make sure to review the doxygen manual and make liberal use of the \a,
400 \code, \c, \b, \note, \li and \e modifiers as appropriate.
401
402 When documenting a 'static' function or an internal structure in a module,
403 use the \internal modifier to ensure that the resulting documentation
404 explicitly says 'for internal use only'.
405
406 Structures should be documented as follows.
407
408 /*!
409  * \brief A very interesting structure.
410  */
411 struct interesting_struct
412 {
413         /*! \brief A data member. */
414         int member1;
415
416         int member2; /*!< \brief Another data member. */
417 }
418
419 Note that /*! */ blocks document the construct immediately following them
420 unless they are written, /*!< */, in which case they document the construct
421 preceding them.