auth/spnego: use better variable names in gensec_spnego_create_negTokenInit()
[sfrench/samba-autobuild/.git] / README.Coding
index ba70a3c53129b97e53303b7e90b4b085768fe0d5..19a363fa1dd13d3f605fc9c009a55fae17cb79ac 100644 (file)
@@ -16,11 +16,16 @@ style should never outweigh coding itself and so the guidelines
 described here are hopefully easy enough to follow as they are very
 common and supported by tools and editors.
 
-The basic style, also mentioned in prog_guide4.txt, is the Linux kernel
+The basic style for C code, also mentioned in prog_guide4.txt, is the Linux kernel
 coding style (See Documentation/CodingStyle in the kernel source tree). This
 closely matches what most Samba developers use already anyways, with a few
 exceptions as mentioned below.
 
+The coding style for Python code is documented in PEP8,
+http://www.python.org/pep/pep8 (with spaces). 
+If you have ever worked on another free software Python project, you are
+probably already familiar with it.
+
 But to save you the trouble of reading the Linux kernel style guide, here
 are the highlights.
 
@@ -85,6 +90,16 @@ displaying trailing whitespace:
   set textwidth=80
   autocmd BufNewFile,BufRead *.c,*.h exec 'match Todo /\%>' . &textwidth . 'v.\+/'
 
+clang-format
+------------
+BasedOnStyle: LLVM
+IndentWidth: 8
+UseTab: true
+BreakBeforeBraces: Linux
+AllowShortIfStatementsOnASingleLine: false
+IndentCaseLabels: false
+BinPackParameters: false
+
 
 =========================
 FAQ & Statement Reference
@@ -96,6 +111,78 @@ Comments
 Comments should always use the standard C syntax.  C++
 style comments are not currently allowed.
 
+The lines before a comment should be empty. If the comment directly
+belongs to the following code, there should be no empty line
+after the comment, except if the comment contains a summary
+of multiple following code blocks.
+
+This is good:
+
+       ...
+       int i;
+
+       /*
+        * This is a multi line comment,
+        * which explains the logical steps we have to do:
+        *
+        * 1. We need to set i=5, because...
+        * 2. We need to call complex_fn1
+        */
+
+       /* This is a one line comment about i = 5. */
+       i = 5;
+
+       /*
+        * This is a multi line comment,
+        * explaining the call to complex_fn1()
+        */
+       ret = complex_fn1();
+       if (ret != 0) {
+       ...
+
+       /**
+        * @brief This is a doxygen comment.
+        *
+        * This is a more detailed explanation of
+        * this simple function.
+        *
+        * @param[in]   param1     The parameter value of the function.
+        *
+        * @param[out]  result1    The result value of the function.
+        *
+        * @return              0 on success and -1 on error.
+        */
+       int example(int param1, int *result1);
+
+This is bad:
+
+       ...
+       int i;
+       /*
+        * This is a multi line comment,
+        * which explains the logical steps we have to do:
+        *
+        * 1. We need to set i=5, because...
+        * 2. We need to call complex_fn1
+        */
+       /* This is a one line comment about i = 5. */
+       i = 5;
+       /*
+        * This is a multi line comment,
+        * explaining the call to complex_fn1()
+        */
+       ret = complex_fn1();
+       if (ret != 0) {
+       ...
+
+       /*This is a one line comment.*/
+
+       /* This is a multi line comment,
+          with some more words...*/
+
+       /*
+        * This is a multi line comment,
+        * with some more words...*/
 
 Indention & Whitespace & 80 columns
 -----------------------------------
@@ -112,7 +199,13 @@ characters or less with whitespace.  For example,
 The previous example is intended to illustrate alignment of function
 parameters across lines and not as encourage for gratuitous line
 splitting.  Never split a line before columns 70 - 79 unless you
-have a really good reason.  Be smart about formatting.
+have a really good reason. Be smart about formatting.
+
+One exception to the previous rule is function declarations and
+definitions. In function declarations and definitions, either the
+declaration is a one-liner, or each parameter is listed on its own
+line. The rationale is that if there are many parameters, each one
+should be on its own line to make tracking interface changes easier.
 
 
 If, switch, & Code blocks
@@ -203,8 +296,8 @@ Good Examples:
                int ret = 0;
 
                if (y < 10) {
-                       z = malloc(sizeof(int)*y);
-                       if (!z) {
+                       z = malloc(sizeof(int) * y);
+                       if (z == NULL) {
                                ret = 1;
                                goto done;
                        }
@@ -213,7 +306,7 @@ Good Examples:
                print("Allocated %d elements.\n", y);
 
         done:
-               if (z) {
+               if (z != NULL) {
                        free(z);
                }
 
@@ -221,25 +314,6 @@ Good Examples:
        }
 
 
-Checking Pointer Values
------------------------
-
-When invoking functions that return pointer values, either of the following
-are acceptable. Use your best judgement and choose the more readable option.
-Remember that many other persons will review it:
-
-       if ((x = malloc(sizeof(short)*10)) == NULL ) {
-               fprintf(stderr, "Unable to alloc memory!\n");
-       }
-
-or:
-
-       x = malloc(sizeof(short)*10);
-       if (!x) {
-               fprintf(stderr, "Unable to alloc memory!\n");
-       }
-
-
 Primitive Data Types
 --------------------
 
@@ -254,6 +328,17 @@ lib/replace/, new code should adhere to the following conventions:
   * Boolean values are "true" and "false" (not True or False)
   * Exact width integers are of type [u]int[8|16|32|64]_t
 
+Most of the time a good name for a boolean variable is 'ok'. Here is an
+example we often use:
+
+       bool ok;
+
+       ok = foo();
+       if (!ok) {
+               /* do something */
+       }
+
+It makes the code more readable and is easy to debug.
 
 Typedefs
 --------
@@ -262,6 +347,39 @@ Samba tries to avoid "typedef struct { .. } x_t;" so we do always try to use
 "struct x { .. };". We know there are still such typedefs in the code,
 but for new code, please don't do that anymore.
 
+Initialize pointers
+-------------------
+
+All pointer variables MUST be initialized to NULL. History has
+demonstrated that uninitialized pointer variables have lead to various
+bugs and security issues.
+
+Pointers MUST be initialized even if the assignment directly follows
+the declaration, like pointer2 in the example below, because the
+instructions sequence may change over time.
+
+Good Example:
+
+       char *pointer1 = NULL;
+       char *pointer2 = NULL;
+
+       pointer2 = some_func2();
+
+       ...
+
+       pointer1 = some_func1();
+
+Bad Example:
+
+       char *pointer1;
+       char *pointer2;
+
+       pointer2 = some_func2();
+
+       ...
+
+       pointer1 = some_func1();
+
 Make use of helper variables
 ----------------------------
 
@@ -271,7 +389,8 @@ it's also easier to use the "step" command within gdb.
 
 Good Example:
 
-       char *name;
+       char *name = NULL;
+       int ret;
 
        name = get_some_name();
        if (name == NULL) {
@@ -287,3 +406,60 @@ Bad Example:
        ret = some_function_my_name(get_some_name());
        ...
 
+Please try to avoid passing function return values to if- or
+while-conditions. The reason for this is better handling of code under a
+debugger.
+
+Good example:
+
+       x = malloc(sizeof(short)*10);
+       if (x == NULL) {
+               fprintf(stderr, "Unable to alloc memory!\n");
+       }
+
+Bad example:
+
+       if ((x = malloc(sizeof(short)*10)) == NULL ) {
+               fprintf(stderr, "Unable to alloc memory!\n");
+       }
+
+There are exceptions to this rule. One example is walking a data structure in
+an iterator style:
+
+       while ((opt = poptGetNextOpt(pc)) != -1) {
+                  ... do something with opt ...
+       }
+
+But in general, please try to avoid this pattern.
+
+
+Control-Flow changing macros
+----------------------------
+
+Macros like NT_STATUS_NOT_OK_RETURN that change control flow
+(return/goto/etc) from within the macro are considered bad, because
+they look like function calls that never change control flow. Please
+do not use them in new code.
+
+The only exception is the test code that depends repeated use of calls
+like CHECK_STATUS, CHECK_VAL and others.
+
+
+DEBUG statements
+----------------
+
+Use these following macros instead of DEBUG:
+
+DBG_ERR        log level 0             error conditions
+DBG_WARNING    log level 1             warning conditions
+DBG_NOTICE     log level 3             normal, but significant, condition
+DBG_INFO       log level 5             informational message
+DBG_DEBUG      log level 10            debug-level message
+
+Example usage:
+
+DBG_ERR("Memory allocation failed\n");
+DBG_DEBUG("Received %d bytes\n", count);
+
+The messages from these macros are automatically prefixed with the
+function name.