Adding some documentation on Asterisk and video telephony. Thanks to the team
[asterisk/asterisk.git] / doc / CODING-GUIDELINES
1 == Asterisk Patch/Coding Guidelines ==
2 --------------------------------------
3
4 We are looking forward to your contributions to Asterisk - the 
5 Open Source PBX! As Asterisk is a large and in some parts very
6 time-sensitive application, the code base needs to conform to
7 a common set of coding rules so that many developers can enhance
8 and maintain the code. Code also needs to be reviewed and tested
9 so that it works and follows the general architecture and guide-
10 lines, and is well documented.
11
12 Asterisk is published under a dual-licensing scheme by Digium.
13 To be accepted into the codebase, all non-trivial changes must be
14 disclaimed to Digium or placed in the public domain. For more information
15 see http://bugs.digium.com
16
17 Patches should be in the form of a unified (-u) diff, made from the directory
18 above the top-level Asterisk source directory. For example:
19
20 - the base code you are working from is in ~/work/asterisk-base
21 - the changes are in ~/work/asterisk-new
22
23 ~/work$ diff -urN asterisk-base asterisk-new
24
25 If you are changing within a fresh CVS/SVN repository, you can create
26 a patch with
27 $ cvs diff -urN <mycodefile>.c
28
29 * General rules
30 ---------------
31
32 - All code, filenames, function names and comments must be in ENGLISH.
33
34 - Don't annotate your changes with comments like "/* JMG 4/20/04 */";
35   Comments should explain what the code does, not when something was changed
36   or who changed it. If you have done a larger contribution, make sure
37   that you are added to the CREDITS file.
38
39 - Don't make unnecessary whitespace changes throughout the code.
40   If you make changes, submit them to the tracker as separate patches
41   that only include whitespace and formatting changes.
42
43 - Don't use C++ type (//) comments.
44
45 - Try to match the existing formatting of the file you are working on.
46
47 - Use spaces instead of tabs when aligning in-line comments or #defines (this makes 
48   your comments aligned even if the code is viewed with another tabsize)
49
50 * Declaration of functions and variables
51 ----------------------------------------
52
53 - Do not declare variables mid-block (e.g. like recent GNU compilers support)
54   since it is harder to read and not portable to GCC 2.95 and others.
55
56 - Functions and variables that are not intended to be used outside the module
57   must be declared static.
58
59 - When reading integer numeric input with scanf (or variants), do _NOT_ use '%i'
60   unless you specifically want to allow non-base-10 input; '%d' is always a better
61   choice, since it will not silently turn numbers with leading zeros into base-8.
62
63 - Strings that are coming from input should not be used as a first argument to
64   a formatted *printf function.
65
66 * Use the internal API
67 ----------------------
68
69 - Make sure you are aware of the string and data handling functions that exist
70   within Asterisk to enhance portability and in some cases to produce more
71   secure and thread-safe code. Check utils.c/utils.h for these.
72
73
74 * Code formatting
75 -----------------
76
77 Roughly, Asterisk code formatting guidelines are generally equivalent to the 
78 following:
79
80 # indent -i4 -ts4 -br -brs -cdw -lp -ce -nbfda -npcs -nprs -npsl -nbbo -saf -sai -saw -cs -l90 foo.c
81
82 this means in verbose:
83  -i4:    indent level 4
84  -ts4:   tab size 4
85  -br:    braces on if line
86  -brs:   braces on struct decl line
87  -cdw:   cuddle do while
88  -lp:    line up continuation below parenthesis
89  -ce:    cuddle else
90  -nbfda: dont break function decl args
91  -npcs:  no space after function call names
92  -nprs:  no space after parentheses
93  -npsl:  dont break procedure type
94  -saf:   space after for
95  -sai:   space after if
96  -saw:   space after while
97  -cs:    space after cast
98  -ln90:  line length 90 columns
99
100 Function calls and arguments should be spaced in a consistent way across
101 the codebase.
102         GOOD: foo(arg1, arg2);
103         GOOD: foo(arg1,arg2);   /* Acceptable but not preferred */
104         BAD: foo (arg1, arg2);
105         BAD: foo( arg1, arg2 );
106         BAD: foo(arg1, arg2,arg3);
107
108 Don't treat keywords (if, while, do, return) as if they were functions;
109 leave space between the keyword and the expression used (if any). For 'return',
110 don't even put parentheses around the expression, since they are not
111 required.
112
113 There is no shortage of whitespace characters :-) Use them when they make
114 the code easier to read. For example:
115
116         for (str=foo;str;str=str->next)
117
118 is harder to read than
119
120         for (str = foo; str; str = str->next)
121
122 Following are examples of how code should be formatted.
123
124 - Functions:
125 int foo(int a, char *s)
126 {
127         return 0;
128 }
129
130 - If statements:
131 if (foo) {
132         bar();
133 } else {
134         blah();
135 }
136
137 - Case statements:
138 switch (foo) {
139 case BAR:
140         blah();
141         break;
142 case OTHER:
143         other();
144         break;
145 }
146
147 - No nested statements without braces, e.g.:
148
149 for (x = 0; x < 5; x++)
150         if (foo) 
151                 if (bar)
152                         baz();
153
154 instead do:
155 for (x = 0; x < 5; x++) {
156         if (foo) {
157                 if (bar)
158                         baz();
159         }
160 }
161
162 - Don't build code like this:
163
164 if (foo) {
165         /* .... 50 lines of code ... */
166 } else {
167         result = 0;
168         return;
169 }
170
171 Instead, try to minimize the number of lines of code that need to be
172 indented, by only indenting the shortest case of the 'if'
173 statement, like so:
174
175 if !(foo) {
176         result = 0;
177         return;
178 }
179
180 .... 50 lines of code ....
181
182 When this technique is used properly, it makes functions much easier to read
183 and follow, especially those with more than one or two 'setup' operations
184 that must succeed for the rest of the function to be able to execute.
185
186 - Labels/goto are acceptable
187 Proper use of this technique may occasionally result in the need for a
188 label/goto combination so that error/failure conditions can exit the
189 function while still performing proper cleanup. This is not a bad thing!
190 Use of goto in this situation is encouraged, since it removes the need
191 for excess code indenting without requiring duplication of cleanup code.
192        
193 - Never use an uninitialized variable
194 Make sure you never use an uninitialized variable.  The compiler will 
195 usually warn you if you do so. However, do not go too far the other way,
196 and needlessly initialize variables that do not require it. If the first
197 time you use a variable in a function is to store a value there, then
198 initializing it at declaration is pointless, and will generate extra
199 object code and data in the resulting binary with no purpose. When in doubt,
200 trust the compiler to tell you when you need to initialize a variable;
201 if it does not warn you, initialization is not needed.
202
203 - Do not cast 'void *'
204 Do not explicitly cast 'void *' into any other type, nor should you cast any
205 other type into 'void *'. Implicit casts to/from 'void *' are explicitly
206 allowed by the C specification. This means the results of malloc(), calloc(),
207 alloca(), and similar functions do not _ever_ need to be cast to a specific
208 type, and when you are passing a pointer to (for example) a callback function
209 that accepts a 'void *' you do not need to cast into that type.
210
211 * Variable naming
212 -----------------
213
214 - Global variables
215 Name global variables (or local variables when you have a lot of them or
216 are in a long function) something that will make sense to aliens who
217 find your code in 100 years.  All variable names should be in lower 
218 case, except when following external APIs or specifications that normally
219 use upper- or mixed-case variable names; in that situation, it is
220 preferable to follow the external API/specification for ease of
221 understanding.
222
223 Make some indication in the name of global variables which represent
224 options that they are in fact intended to be global.
225  e.g.: static char global_something[80]
226
227 - Don't use un-necessary typedef's
228 Don't use 'typedef' just to shorten the amount of typing; there is no substantial
229 benefit in this:
230
231 struct foo {
232         int bar;
233 };
234 typedef foo_t struct foo;
235
236 In fact, don't use 'variable type' suffixes at all; it's much preferable to
237 just type 'struct foo' rather than 'foo_s'.
238
239 - Use enums instead of #define where possible
240 Use enums rather than long lists of #define-d numeric constants when possible;
241 this allows structure members, local variables and function arguments to
242 be declared as using the enum's type. For example:
243
244 enum option {
245   OPT_FOO = 1
246   OPT_BAR = 2
247   OPT_BAZ = 4
248 };
249
250 static enum option global_option;
251
252 static handle_option(const enum option opt)
253 {
254   ...
255 }
256
257 Note: The compiler will _not_ force you to pass an entry from the enum
258 as an argument to this function; this recommendation serves only to make
259 the code clearer and somewhat self-documenting. In addition, when using
260 switch/case blocks that switch on enum values, the compiler will warn
261 you if you forget to handle one or more of the enum values, which can be
262 handy.
263
264 * String handling
265 -----------------
266
267 Don't use strncpy for copying whole strings; it does not guarantee that the
268 output buffer will be null-terminated. Use ast_copy_string instead, which
269 is also slightly more efficient (and allows passing the actual buffer
270 size, which makes the code clearer).
271
272 Don't use ast_copy_string (or any length-limited copy function) for copying
273 fixed (known at compile time) strings into buffers, if the buffer is something
274 that has been allocated in the function doing the copying. In that case, you
275 know at the time you are writing the code whether the buffer is large enough
276 for the fixed string or not, and if it's not, your code won't work anyway!
277 Use strcpy() for this operation, or directly set the first two characters
278 of the buffer if you are just trying to store a one-character string in the
279 buffer. If you are trying to 'empty' the buffer, just store a single
280 NULL character ('\0') in the first byte of the buffer; nothing else is
281 needed, and any other method is wasteful.
282
283 In addition, if the previous operations in the function have already
284 determined that the buffer in use is adequately sized to hold the string
285 you wish to put into it (even if you did not allocate the buffer yourself),
286 use a direct strcpy(), as it can be inlined and optimized to simple
287 processor operations, unlike ast_copy_string().
288
289 * Use of functions
290 ------------------
291
292 When making applications, always ast_strdupa(data) to a local pointer if
293 you intend to parse the incoming data string.
294
295         if (data)
296                 mydata = ast_strdupa(data);
297
298
299 - Separating arguments to dialplan applications and functions
300 Use ast_app_separate_args() to separate the arguments to your application
301 once you have made a local copy of the string.
302
303 - Parsing strings with strsep
304 Use strsep() for parsing strings when possible; there is no worry about
305 're-entrancy' as with strtok(), and even though it modifies the original
306 string (which the man page warns about), in many cases that is exactly
307 what you want!
308
309 - Create generic code!
310 If you do the same or a similar operation more than one time, make it a
311 function or macro.
312
313 Make sure you are not duplicating any functionality already found in an
314 API call somewhere.  If you are duplicating functionality found in 
315 another static function, consider the value of creating a new API call 
316 which can be shared.
317
318 * Handling of pointers and allocations
319 --------------------------------------
320
321 - Dereference or localize pointers
322 Always dereference or localize pointers to things that are not yours like
323 channel members in a channel that is not associated with the current 
324 thread and for which you do not have a lock.
325         channame = ast_strdupa(otherchan->name);
326
327 - Use const on pointer arguments if possible
328 Use const on pointer arguments which your function will not be modifying, as this 
329 allows the compiler to make certain optimizations. In general, use 'const'
330 on any argument that you have no direct intention of modifying, as it can
331 catch logic/typing errors in your code when you use the argument variable
332 in a way that you did not intend.
333
334 - Do not create your own linked list code - reuse!
335 As a common example of this point, make an effort to use the lockable
336 linked-list macros found in include/asterisk/linkedlists.h. They are
337 efficient, easy to use and provide every operation that should be
338 necessary for managing a singly-linked list (if something is missing,
339 let us know!). Just because you see other open-coded list implementations
340 in the source tree is no reason to continue making new copies of
341 that code... There are also a number of common string manipulation
342 and timeval manipulation functions in asterisk/strings.h and asterisk/time.h;
343 use them when possible.
344
345 - Avoid needless allocations!
346 Avoid needless malloc(), strdup() calls. If you only need the value in
347 the scope of your function try ast_strdupa() or declare structs on the
348 stack and pass a pointer to them. However, be careful to _never_ call
349 alloca(), ast_strdupa() or similar functions in the argument list
350 of a function you are calling; this can cause very strange stack
351 arrangements and produce unexpected behavior.
352
353 -Allocations for structures
354 When allocating/zeroing memory for a structure, use code like this:
355
356 struct foo *tmp;
357
358 ...
359
360 tmp = ast_calloc(1, sizeof(*tmp));
361
362 Avoid the combination of ast_malloc() and memset().  Instead, always use
363 ast_calloc(). This will allocate and zero the memory in a single operation. 
364 In the case that uninitialized memory is acceptable, there should be a comment
365 in the code that states why this is the case.
366
367 Using sizeof(*tmp) instead of sizeof(struct foo) eliminates duplication of the 
368 'struct foo' identifier, which makes the code easier to read and also ensures 
369 that if it is copy-and-pasted it won't require as much editing.
370
371 The ast_* family of functions for memory allocation are functionally the same.
372 They just add an Asterisk log error message in the case that the allocation
373 fails for some reason. This eliminates the need to generate custom messages
374 throughout the code to log that this has occurred.
375
376 -String Duplications
377
378 The functions strdup and strndup can *not* accept a NULL argument. This results
379 in having code like this:
380
381         if (str)
382                 newstr = strdup(str);
383         else
384                 newstr = NULL;
385
386 However, the ast_strdup and ast_strdup functions will happily accept a NULL
387 argument without generating an error.  The same code can be written as:
388         
389         newstr = ast_strdup(str);
390
391 Furthermore, it is unnecessary to have code that malloc/calloc's for the length
392 of a string (+1 for the terminating '\0') and then using strncpy to copy the
393 copy the string into the resulting buffer.  This is the exact same thing as
394 using ast_strdup.
395
396 * CLI Commands
397 --------------
398
399 New CLI commands should be named using the module's name, followed by a verb
400 and then any parameters that the command needs. For example:
401
402 *CLI> iax2 show peer <peername>
403
404 not
405
406 *CLI> show iax2 peer <peername>
407
408 * New dialplan applications/functions
409 -------------------------------------
410
411 There are two methods of adding functionality to the Asterisk
412 dialplan: applications and functions. Applications (found generally in
413 the apps/ directory) should be collections of code that interact with
414 a channel and/or user in some significant way. Functions (which can be
415 provided by any type of module) are used when the provided
416 functionality is simple... getting/retrieving a value, for
417 example. Functions should also be used when the operation is in no way
418 related to a channel (a computation or string operation, for example).
419
420 Applications are registered and invoked using the
421 ast_register_application function; see the apps/app_skel.c file for an
422 example.
423
424 Functions are registered using 'struct ast_custom_function'
425 structures and the ast_custom_function_register function.
426
427 * Doxygen API Documentation Guidelines
428 --------------------------------------
429
430 When writing Asterisk API documentation the following format should be
431 followed. Do not use the javadoc style.
432
433 /*!
434  * \brief Do interesting stuff.
435  * \param thing1 interesting parameter 1.
436  * \param thing2 interesting parameter 2.
437  *
438  * This function does some interesting stuff.
439  *
440  * \return zero on success, -1 on error.
441  */
442 int ast_interesting_stuff(int thing1, int thing2)
443 {
444         return 0;
445 }
446
447 Notice the use of the \param, \brief, and \return constructs.  These should be
448 used to describe the corresponding pieces of the function being documented.
449 Also notice the blank line after the last \param directive.  All doxygen
450 comments must be in one /*! */ block.  If the function or struct does not need
451 an extended description it can be left out.
452
453 Please make sure to review the doxygen manual and make liberal use of the \a,
454 \code, \c, \b, \note, \li and \e modifiers as appropriate.
455
456 When documenting a 'static' function or an internal structure in a module,
457 use the \internal modifier to ensure that the resulting documentation
458 explicitly says 'for internal use only'.
459
460 Structures should be documented as follows.
461
462 /*!
463  * \brief A very interesting structure.
464  */
465 struct interesting_struct
466 {
467         /*! \brief A data member. */
468         int member1;
469
470         int member2; /*!< \brief Another data member. */
471 }
472
473 Note that /*! */ blocks document the construct immediately following them
474 unless they are written, /*!< */, in which case they document the construct
475 preceding them.
476
477 * Finishing up before you submit your code
478 ------------------------------------------
479
480 - Look at the code once more
481 When you achieve your desired functionalty, make another few refactor
482 passes over the code to optimize it.
483
484 - Read the patch
485 Before submitting a patch, *read* the actual patch file to be sure that 
486 all the changes you expect to be there are, and that there are no 
487 surprising changes you did not expect. During your development, that
488 part of Asterisk may have changed, so make sure you compare with the
489 latest CVS.
490
491 - Listen to advice
492 If you are asked to make changes to your patch, there is a good chance
493 the changes will introduce bugs, check it even more at this stage.
494 Also remember that the bug marshal or co-developer that adds comments 
495 is only human, they may be in error :-)
496
497 - Optimize, optimize, optimize
498 If you are going to reuse a computed value, save it in a variable
499 instead of recomputing it over and over.  This can prevent you from 
500 making a mistake in subsequent computations, making it easier to correct
501 if the formula has an error and may or may not help optimization but 
502 will at least help readability.
503
504 Just an example (so don't over analyze it, that'd be a shame):
505
506 const char *prefix = "pre";     
507 const char *postfix = "post";
508 char *newname;
509 char *name = "data";
510
511 if (name && (newname = alloca(strlen(name) + strlen(prefix) + strlen(postfix) + 3)))
512         snprintf(newname, strlen(name) + strlen(prefix) + strlen(postfix) + 3, "%s/%s/%s", prefix, name, postfix);
513
514 ...vs this alternative:
515
516 const char *prefix = "pre";
517 const char *postfix = "post";
518 char *newname;
519 char *name = "data";
520 int len = 0;
521
522 if (name && (len = strlen(name) + strlen(prefix) + strlen(postfix) + 3) && (newname = alloca(len)))
523         snprintf(newname, len, "%s/%s/%s", prefix, name, postfix);
524
525 * Creating new manager events?
526 ------------------------------
527 If you create new AMI events, please read manager.txt. Do not re-use
528 existing headers for new purposes, but please re-use existing headers
529 for the same type of data.
530
531 Manager events that signal a status are required to have one
532 event name, with a status header that shows the status.
533 The old style, with one event named "ThisEventOn" and another named
534 "ThisEventOff", is no longer approved.
535
536 Check manager.txt for more information on manager and existing
537 headers. Please update this file if you add new headers.
538
539 -----------------------------------------------
540 Welcome to the Asterisk development community!
541 Meet you on the asterisk-dev mailing list. 
542 Subscribe at http://lists.digium.com!
543
544 Mark Spencer, Kevin P. Fleming and 
545 the Asterisk.org Development Team