mirror of https://github.com/asterisk/asterisk
You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
294 lines
8.7 KiB
294 lines
8.7 KiB
== 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.
|