Add in check for the GCC attribute deprecated. It may be used soon!
[asterisk/asterisk.git] / doc / CODING-GUIDELINES
old mode 100755 (executable)
new mode 100644 (file)
index 3d296a7..990f5bc
@@ -14,17 +14,15 @@ 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:
+Patches should be in the form of a unified (-u) diff, made from a checkout
+from subversion.
 
-- the base code you are working from is in ~/work/asterisk-base
-- the changes are in ~/work/asterisk-new
+/usr/src/asterisk$ svn diff > mypatch
 
-~/work$ diff -urN asterisk-base asterisk-new
+If you would like to only include changes to certain files in the patch, you
+can list them in the "svn diff" command:
 
-If you are changing within a fresh CVS/SVN repository, you can create
-a patch with
-$ cvs diff -urN <mycodefile>.c
+/usr/src/asterisk$ svn diff somefile.c someotherfile.c > mypatch
 
 * General rules
 ---------------
@@ -70,6 +68,10 @@ $ cvs diff -urN <mycodefile>.c
   within Asterisk to enhance portability and in some cases to produce more
   secure and thread-safe code. Check utils.c/utils.h for these.
 
+- If you need to create a detached thread, use the ast_pthread_create_detached()
+  normally or ast_pthread_create_detached_background() for a thread with a smaller
+  stack size.  This reduces the replication of the code to handle the pthread_attr_t
+  structure.
 
 * Code formatting
 -----------------
@@ -77,7 +79,7 @@ $ cvs diff -urN <mycodefile>.c
 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
+# 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
@@ -85,7 +87,7 @@ this means in verbose:
  -br:    braces on if line
  -brs:   braces on struct decl line
  -cdw:   cuddle do while
- -cli0:  case indentation 0
+ -lp:    line up continuation below parenthesis
  -ce:    cuddle else
  -nbfda: dont break function decl args
  -npcs:  no space after function call names
@@ -94,6 +96,8 @@ this means in verbose:
  -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.
@@ -170,7 +174,7 @@ 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) {
+if (!foo) {
        result = 0;
        return;
 }
@@ -206,6 +210,18 @@ 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.
 
+* Function naming
+-----------------
+
+All public functions (those not marked 'static'), must be named "ast_<something>"
+and have a descriptive name.
+
+As an example, suppose you wanted to take a local function "find_feature", defined
+as static in a file, and used only in that file, and make it public, and use it
+in other files. You will have to remove the "static" declaration and define a 
+prototype in an appropriate header file (usually in include/asterisk). A more
+specific name should be given, such as "ast_find_call_feature".
+
 * Variable naming
 -----------------
 
@@ -225,11 +241,7 @@ options that they are in fact intended to be global.
 - 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;
+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'.
@@ -349,27 +361,47 @@ 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:
+When allocating/zeroing memory for a structure, use code like this:
 
 struct foo *tmp;
 
 ...
 
-tmp = malloc(sizeof(*tmp));
-if (tmp)
-       memset(tmp, 0, sizeof(*tmp));
+tmp = ast_calloc(1, 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:
+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.
 
-struct foo *tmp;
+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;
 
-tmp = calloc(1, sizeof(*tmp));
+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);
 
-This will allocate and zero the memory in a single operation.
+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
 --------------
@@ -452,11 +484,19 @@ Note that /*! */ blocks document the construct immediately following them
 unless they are written, /*!< */, in which case they document the construct
 preceding them.
 
+It is very much preferred that documentation is not done inline, as done in
+the previous example for member2.  The first reason for this is that it tends
+to encourage extremely brief, and often pointless, documentation since people
+try to keep the comment from making the line extremely long.  However, if you
+insist on using inline comments, please indent the documentation with spaces!
+That way, all of the comments are properly aligned, regardless of what tab
+size is being used for viewing the code.
+
 * Finishing up before you submit your code
 ------------------------------------------
 
 - Look at the code once more
-When you achieve your desired functionalty, make another few refactor
+When you achieve your desired functionality, make another few refactor
 passes over the code to optimize it.
 
 - Read the patch
@@ -500,6 +540,19 @@ 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!