diff options
Diffstat (limited to '1.4.23-rc4/doc/CODING-GUIDELINES')
| -rw-r--r-- | 1.4.23-rc4/doc/CODING-GUIDELINES | 543 |
1 files changed, 0 insertions, 543 deletions
diff --git a/1.4.23-rc4/doc/CODING-GUIDELINES b/1.4.23-rc4/doc/CODING-GUIDELINES deleted file mode 100644 index c3ffacd3c..000000000 --- a/1.4.23-rc4/doc/CODING-GUIDELINES +++ /dev/null @@ -1,543 +0,0 @@ -== Asterisk Patch/Coding Guidelines == --------------------------------------- - -We are looking forward to your contributions to Asterisk - the -Open Source PBX! As Asterisk is a large and in some parts very -time-sensitive application, the code base needs to conform to -a common set of coding rules so that many developers can enhance -and maintain the code. Code also needs to be reviewed and tested -so that it works and follows the general architecture and guide- -lines, and is well documented. - -Asterisk is published under a dual-licensing scheme by Digium. -To be accepted into the codebase, all non-trivial changes must be -disclaimed to Digium or placed in the public domain. For more information -see http://bugs.digium.com - -Patches should be in the form of a unified (-u) diff, made from a checkout -from subversion. - -/usr/src/asterisk$ svn diff > mypatch - -If you would like to only include changes to certain files in the patch, you -can list them in the "svn diff" command: - -/usr/src/asterisk$ svn diff somefile.c someotherfile.c > mypatch - -* General rules ---------------- - -- All code, filenames, function names and comments must be in ENGLISH. - -- Don't annotate your changes with comments like "/* JMG 4/20/04 */"; - Comments should explain what the code does, not when something was changed - or who changed it. If you have done a larger contribution, make sure - that you are added to the CREDITS file. - -- Don't make unnecessary whitespace changes throughout the code. - If you make changes, submit them to the tracker as separate patches - that only include whitespace and formatting changes. - -- Don't use C++ type (//) comments. - -- Try to match the existing formatting of the file you are working on. - -- Use spaces instead of tabs when aligning in-line comments or #defines (this makes - your comments aligned even if the code is viewed with another tabsize) - -* Declaration of functions and variables ----------------------------------------- - -- Do not declare variables mid-block (e.g. like recent GNU compilers support) - since it is harder to read and not portable to GCC 2.95 and others. - -- Functions and variables that are not intended to be used outside the module - must be declared static. - -- When reading integer numeric input with scanf (or variants), do _NOT_ use '%i' - unless you specifically want to allow non-base-10 input; '%d' is always a better - choice, since it will not silently turn numbers with leading zeros into base-8. - -- Strings that are coming from input should not be used as a first argument to - a formatted *printf function. - -* Use the internal API ----------------------- - -- Make sure you are aware of the string and data handling functions that exist - within Asterisk to enhance portability and in some cases to produce more - secure and thread-safe code. Check utils.c/utils.h for these. - - -* Code formatting ------------------ - -Roughly, Asterisk code formatting guidelines are generally equivalent to the -following: - -# indent -i4 -ts4 -br -brs -cdw -lp -ce -nbfda -npcs -nprs -npsl -nbbo -saf -sai -saw -cs -l90 foo.c - -this means in verbose: - -i4: indent level 4 - -ts4: tab size 4 - -br: braces on if line - -brs: braces on struct decl line - -cdw: cuddle do while - -lp: line up continuation below parenthesis - -ce: cuddle else - -nbfda: dont break function decl args - -npcs: no space after function call names - -nprs: no space after parentheses - -npsl: dont break procedure type - -saf: space after for - -sai: space after if - -saw: space after while - -cs: space after cast - -ln90: line length 90 columns - -Function calls and arguments should be spaced in a consistent way across -the codebase. - GOOD: foo(arg1, arg2); - GOOD: foo(arg1,arg2); /* Acceptable but not preferred */ - BAD: foo (arg1, arg2); - BAD: foo( arg1, arg2 ); - BAD: foo(arg1, arg2,arg3); - -Don't treat keywords (if, while, do, return) as if they were functions; -leave space between the keyword and the expression used (if any). For 'return', -don't even put parentheses around the expression, since they are not -required. - -There is no shortage of whitespace characters :-) Use them when they make -the code easier to read. For example: - - for (str=foo;str;str=str->next) - -is harder to read than - - for (str = foo; str; str = str->next) - -Following are examples of how code should be formatted. - -- Functions: -int foo(int a, char *s) -{ - return 0; -} - -- If statements: -if (foo) { - bar(); -} else { - blah(); -} - -- Case statements: -switch (foo) { -case BAR: - blah(); - break; -case OTHER: - other(); - break; -} - -- No nested statements without braces, e.g.: - -for (x = 0; x < 5; x++) - if (foo) - if (bar) - baz(); - -instead do: -for (x = 0; x < 5; x++) { - if (foo) { - if (bar) - baz(); - } -} - -- Don't build code like this: - -if (foo) { - /* .... 50 lines of code ... */ -} else { - result = 0; - return; -} - -Instead, try to minimize the number of lines of code that need to be -indented, by only indenting the shortest case of the 'if' -statement, like so: - -if (!foo) { - result = 0; - return; -} - -.... 50 lines of code .... - -When this technique is used properly, it makes functions much easier to read -and follow, especially those with more than one or two 'setup' operations -that must succeed for the rest of the function to be able to execute. - -- Labels/goto are acceptable -Proper use of this technique may occasionally result in the need for a -label/goto combination so that error/failure conditions can exit the -function while still performing proper cleanup. This is not a bad thing! -Use of goto in this situation is encouraged, since it removes the need -for excess code indenting without requiring duplication of cleanup code. - -- Never use an uninitialized variable -Make sure you never use an uninitialized variable. The compiler will -usually warn you if you do so. However, do not go too far the other way, -and needlessly initialize variables that do not require it. If the first -time you use a variable in a function is to store a value there, then -initializing it at declaration is pointless, and will generate extra -object code and data in the resulting binary with no purpose. When in doubt, -trust the compiler to tell you when you need to initialize a variable; -if it does not warn you, initialization is not needed. - -- Do not cast 'void *' -Do not explicitly cast 'void *' into any other type, nor should you cast any -other type into 'void *'. Implicit casts to/from 'void *' are explicitly -allowed by the C specification. This means the results of malloc(), calloc(), -alloca(), and similar functions do not _ever_ need to be cast to a specific -type, and when you are passing a pointer to (for example) a callback function -that accepts a 'void *' you do not need to cast into that type. - -* Variable naming ------------------ - -- Global variables -Name global variables (or local variables when you have a lot of them or -are in a long function) something that will make sense to aliens who -find your code in 100 years. All variable names should be in lower -case, except when following external APIs or specifications that normally -use upper- or mixed-case variable names; in that situation, it is -preferable to follow the external API/specification for ease of -understanding. - -Make some indication in the name of global variables which represent -options that they are in fact intended to be global. - e.g.: static char global_something[80] - -- Don't use un-necessary typedef's -Don't use 'typedef' just to shorten the amount of typing; there is no substantial -benefit in this: - -struct foo { - int bar; -}; -typedef struct foo foo_t; - -In fact, don't use 'variable type' suffixes at all; it's much preferable to -just type 'struct foo' rather than 'foo_s'. - -- Use enums instead of #define where possible -Use enums rather than long lists of #define-d numeric constants when possible; -this allows structure members, local variables and function arguments to -be declared as using the enum's type. For example: - -enum option { - OPT_FOO = 1 - OPT_BAR = 2 - OPT_BAZ = 4 -}; - -static enum option global_option; - -static handle_option(const enum option opt) -{ - ... -} - -Note: The compiler will _not_ force you to pass an entry from the enum -as an argument to this function; this recommendation serves only to make -the code clearer and somewhat self-documenting. In addition, when using -switch/case blocks that switch on enum values, the compiler will warn -you if you forget to handle one or more of the enum values, which can be -handy. - -* String handling ------------------ - -Don't use strncpy for copying whole strings; it does not guarantee that the -output buffer will be null-terminated. Use ast_copy_string instead, which -is also slightly more efficient (and allows passing the actual buffer -size, which makes the code clearer). - -Don't use ast_copy_string (or any length-limited copy function) for copying -fixed (known at compile time) strings into buffers, if the buffer is something -that has been allocated in the function doing the copying. In that case, you -know at the time you are writing the code whether the buffer is large enough -for the fixed string or not, and if it's not, your code won't work anyway! -Use strcpy() for this operation, or directly set the first two characters -of the buffer if you are just trying to store a one-character string in the -buffer. If you are trying to 'empty' the buffer, just store a single -NULL character ('\0') in the first byte of the buffer; nothing else is -needed, and any other method is wasteful. - -In addition, if the previous operations in the function have already -determined that the buffer in use is adequately sized to hold the string -you wish to put into it (even if you did not allocate the buffer yourself), -use a direct strcpy(), as it can be inlined and optimized to simple -processor operations, unlike ast_copy_string(). - -* Use of functions ------------------- - -When making applications, always ast_strdupa(data) to a local pointer if -you intend to parse the incoming data string. - - if (data) - mydata = ast_strdupa(data); - - -- Separating arguments to dialplan applications and functions -Use ast_app_separate_args() to separate the arguments to your application -once you have made a local copy of the string. - -- Parsing strings with strsep -Use strsep() for parsing strings when possible; there is no worry about -'re-entrancy' as with strtok(), and even though it modifies the original -string (which the man page warns about), in many cases that is exactly -what you want! - -- Create generic code! -If you do the same or a similar operation more than one time, make it a -function or macro. - -Make sure you are not duplicating any functionality already found in an -API call somewhere. If you are duplicating functionality found in -another static function, consider the value of creating a new API call -which can be shared. - -* Handling of pointers and allocations --------------------------------------- - -- Dereference or localize pointers -Always dereference or localize pointers to things that are not yours like -channel members in a channel that is not associated with the current -thread and for which you do not have a lock. - channame = ast_strdupa(otherchan->name); - -- Use const on pointer arguments if possible -Use const on pointer arguments which your function will not be modifying, as this -allows the compiler to make certain optimizations. In general, use 'const' -on any argument that you have no direct intention of modifying, as it can -catch logic/typing errors in your code when you use the argument variable -in a way that you did not intend. - -- Do not create your own linked list code - reuse! -As a common example of this point, make an effort to use the lockable -linked-list macros found in include/asterisk/linkedlists.h. They are -efficient, easy to use and provide every operation that should be -necessary for managing a singly-linked list (if something is missing, -let us know!). Just because you see other open-coded list implementations -in the source tree is no reason to continue making new copies of -that code... There are also a number of common string manipulation -and timeval manipulation functions in asterisk/strings.h and asterisk/time.h; -use them when possible. - -- Avoid needless allocations! -Avoid needless malloc(), strdup() calls. If you only need the value in -the scope of your function try ast_strdupa() or declare structs on the -stack and pass a pointer to them. However, be careful to _never_ call -alloca(), ast_strdupa() or similar functions in the argument list -of a function you are calling; this can cause very strange stack -arrangements and produce unexpected behavior. - --Allocations for structures -When allocating/zeroing memory for a structure, use code like this: - -struct foo *tmp; - -... - -tmp = ast_calloc(1, sizeof(*tmp)); - -Avoid the combination of ast_malloc() and memset(). Instead, always use -ast_calloc(). This will allocate and zero the memory in a single operation. -In the case that uninitialized memory is acceptable, there should be a comment -in the code that states why this is the case. - -Using sizeof(*tmp) instead of sizeof(struct foo) eliminates duplication of the -'struct foo' identifier, which makes the code easier to read and also ensures -that if it is copy-and-pasted it won't require as much editing. - -The ast_* family of functions for memory allocation are functionally the same. -They just add an Asterisk log error message in the case that the allocation -fails for some reason. This eliminates the need to generate custom messages -throughout the code to log that this has occurred. - --String Duplications - -The functions strdup and strndup can *not* accept a NULL argument. This results -in having code like this: - - if (str) - newstr = strdup(str); - else - newstr = NULL; - -However, the ast_strdup and ast_strdup functions will happily accept a NULL -argument without generating an error. The same code can be written as: - - newstr = ast_strdup(str); - -Furthermore, it is unnecessary to have code that malloc/calloc's for the length -of a string (+1 for the terminating '\0') and then using strncpy to copy the -copy the string into the resulting buffer. This is the exact same thing as -using ast_strdup. - -* CLI Commands --------------- - -New CLI commands should be named using the module's name, followed by a verb -and then any parameters that the command needs. For example: - -*CLI> iax2 show peer <peername> - -not - -*CLI> show iax2 peer <peername> - -* New dialplan applications/functions -------------------------------------- - -There are two methods of adding functionality to the Asterisk -dialplan: applications and functions. Applications (found generally in -the apps/ directory) should be collections of code that interact with -a channel and/or user in some significant way. Functions (which can be -provided by any type of module) are used when the provided -functionality is simple... getting/retrieving a value, for -example. Functions should also be used when the operation is in no way -related to a channel (a computation or string operation, for example). - -Applications are registered and invoked using the -ast_register_application function; see the apps/app_skel.c file for an -example. - -Functions are registered using 'struct ast_custom_function' -structures and the ast_custom_function_register function. - -* Doxygen API Documentation Guidelines --------------------------------------- - -When writing Asterisk API documentation the following format should be -followed. Do not use the javadoc style. - -/*! - * \brief Do interesting stuff. - * \param thing1 interesting parameter 1. - * \param thing2 interesting parameter 2. - * - * This function does some interesting stuff. - * - * \return zero on success, -1 on error. - */ -int ast_interesting_stuff(int thing1, int thing2) -{ - return 0; -} - -Notice the use of the \param, \brief, and \return constructs. These should be -used to describe the corresponding pieces of the function being documented. -Also notice the blank line after the last \param directive. All doxygen -comments must be in one /*! */ block. If the function or struct does not need -an extended description it can be left out. - -Please make sure to review the doxygen manual and make liberal use of the \a, -\code, \c, \b, \note, \li and \e modifiers as appropriate. - -When documenting a 'static' function or an internal structure in a module, -use the \internal modifier to ensure that the resulting documentation -explicitly says 'for internal use only'. - -Structures should be documented as follows. - -/*! - * \brief A very interesting structure. - */ -struct interesting_struct -{ - /*! \brief A data member. */ - int member1; - - int member2; /*!< \brief Another data member. */ -} - -Note that /*! */ blocks document the construct immediately following them -unless they are written, /*!< */, in which case they document the construct -preceding them. - -* Finishing up before you submit your code ------------------------------------------- - -- Look at the code once more -When you achieve your desired functionality, make another few refactor -passes over the code to optimize it. - -- Read the patch -Before submitting a patch, *read* the actual patch file to be sure that -all the changes you expect to be there are, and that there are no -surprising changes you did not expect. During your development, that -part of Asterisk may have changed, so make sure you compare with the -latest CVS. - -- Listen to advice -If you are asked to make changes to your patch, there is a good chance -the changes will introduce bugs, check it even more at this stage. -Also remember that the bug marshal or co-developer that adds comments -is only human, they may be in error :-) - -- Optimize, optimize, optimize -If you are going to reuse a computed value, save it in a variable -instead of recomputing it over and over. This can prevent you from -making a mistake in subsequent computations, making it easier to correct -if the formula has an error and may or may not help optimization but -will at least help readability. - -Just an example (so don't over analyze it, that'd be a shame): - -const char *prefix = "pre"; -const char *postfix = "post"; -char *newname; -char *name = "data"; - -if (name && (newname = alloca(strlen(name) + strlen(prefix) + strlen(postfix) + 3))) - snprintf(newname, strlen(name) + strlen(prefix) + strlen(postfix) + 3, "%s/%s/%s", prefix, name, postfix); - -...vs this alternative: - -const char *prefix = "pre"; -const char *postfix = "post"; -char *newname; -char *name = "data"; -int len = 0; - -if (name && (len = strlen(name) + strlen(prefix) + strlen(postfix) + 3) && (newname = alloca(len))) - snprintf(newname, len, "%s/%s/%s", prefix, name, postfix); - -* Creating new manager events? ------------------------------- -If you create new AMI events, please read manager.txt. Do not re-use -existing headers for new purposes, but please re-use existing headers -for the same type of data. - -Manager events that signal a status are required to have one -event name, with a status header that shows the status. -The old style, with one event named "ThisEventOn" and another named -"ThisEventOff", is no longer approved. - -Check manager.txt for more information on manager and existing -headers. Please update this file if you add new headers. - ------------------------------------------------ -Welcome to the Asterisk development community! -Meet you on the asterisk-dev mailing list. -Subscribe at http://lists.digium.com! - -Mark Spencer, Kevin P. Fleming and -the Asterisk.org Development Team |