path: root/doc/CODING-GUIDELINES

== Asterisk Patch/Coding Guidelines ==

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

All code, filenames, function names and comments must be in ENGLISH.

Do not declare variables mid-function (e.g. like GNU lets you) since it is
harder to read and not portable to GCC 2.95 and others.

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.

Don't make unnecessary whitespace changes throughout the code.

Don't use C++ type (//) comments.

Try to match the existing formatting of the file you are working on.

Functions and variables that are not intended to be global must be
declared static.

When reading integer numeric input with scanf (or variants), do _NOT_ use '%i'
unless 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.

Roughly, Asterisk code formatting guidelines are generally equivalent to the 
following:

# indent -i4 -ts4 -br -brs -cdw -cli0 -ce -nbfda -npcs -npsl foo.c

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

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. no:

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.
       
Make sure you never use an uninitialized variable.  The compiler will 
usually warn you if you do so.

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.

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]

When making applications, always ast_strdupa(data) to a local pointer if
you intend to parse it.

	if (data)
		mydata = ast_strdupa(data);

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

If you do the same or a similar operation more than 1 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.

When you achieve your desired functionalty, make another few refactor
passes over the code to optimize it.

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.

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.

Avoid needless malloc(),strdup() calls.  If you only need the value in
the scope of your function try ast_strdupa() or declare struts static
and pass them as a pointer with &.

If you are going to reuse a computable value, save it in a variable
instead of recomputing it over and over.  This can prevent you from 
making a mistake in subsequent computations, make 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 = NULL;
char *name = "data";

if (name && (newname = (char *) alloca(strlen(name) + strlen(prefix) + strlen(postfix) + 3)))
	snprintf(newname, strlen(name) + strlen(prefix) + strlen(postfix) + 3, "%s/%s/%s", prefix, name, postfix);

vs

const char *prefix = "pre";
const char *postfix = "post";
char *newname = NULL;
char *name = "data";
int len = 0;

if (name && (len = strlen(name) + strlen(prefix) + strlen(postfix) + 3) && (newname = (char *) alloca(len)))
	snprintf(newname, len, "%s/%s/%s", prefix, name, postfix);


Use const on pointers which your function will not be modifying, as this 
allows the compiler to make certain optimizations.

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

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.

/*!
 * \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.