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