Blocked revisions 74262 via svnmerge
[asterisk/asterisk.git] / doc / CODING-GUIDELINES
index 42d54ac..990f5bc 100644 (file)
@@ -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'.
@@ -384,8 +396,12 @@ in having code like this:
 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 = strdup(str);
+       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
 --------------
@@ -468,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
@@ -516,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!