Asterisk.org: Developer Documentation (28 Oct 2009)

Wed Oct 28 15:49:24 2009

## Asterisk developer's documentation

# Coding Guidelines

## Guidelines

This file is in the /doc directory in your Asterisk source tree. Make sure to stay up to date with the latest guidelines.

    == 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 the directory
    above the top-level Asterisk source directory. For example:

    - the base code you are working from is in ~/work/asterisk-base
    - the changes are in ~/work/asterisk-new

    ~/work$ diff -urN asterisk-base asterisk-new

    If you are changing within a fresh SVN working copy, you can create
    a patch with
    $ svn diff <mycodefile>.c

    * 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 -cli0 -ce -nbfda -npcs -nprs -npsl -saf -sai -saw 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
     -cli0:  case indentation 0
     -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

    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 foo_t struct foo;

    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, try to use code like this:

    struct foo *tmp;

    ...

    tmp = malloc(sizeof(*tmp));
    if (tmp)
    	memset(tmp, 0, sizeof(*tmp));

    This 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. In fact, you can even use:

    struct foo *tmp;

    ...

    tmp = calloc(1, sizeof(*tmp));

    This will allocate and zero the memory in a single operation.

    * 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 functionalty, 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 SVN.

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

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