| |
@@ -0,0 +1,579 @@
|
| |
+ Coding guidelines
|
| |
+ =================
|
| |
+
|
| |
+ Introduction
|
| |
+ ------------
|
| |
+
|
| |
+ This manual contains coding standards and guidelines for SSSD and
|
| |
+ FreeIPA contributors. It is updated based heavily on the FreeIPA style
|
| |
+ which was based on `389 Directory Server style
|
| |
+ guide <http://www.port389.org/docs/389ds/development/coding-style.html>`__
|
| |
+ but deviates from it and covers more aspects.
|
| |
+
|
| |
+ All new code should adhere to these standards but please do not go back
|
| |
+ and make wholesale formatting changes to the old code. It just confuses
|
| |
+ things and is generally a waste of time.
|
| |
+
|
| |
+ Why Coding Guidelines?
|
| |
+ ----------------------
|
| |
+
|
| |
+ The Coding Guidelines are necessary to improve maintainability and
|
| |
+ readability of the code.
|
| |
+
|
| |
+ Maintainability
|
| |
+ ^^^^^^^^^^^^^^^
|
| |
+
|
| |
+ If you're merging changes from a patch it's much easier if everyone is
|
| |
+ using the same coding style. This isn't the reality for a lot of our
|
| |
+ code, but we're trying to get to the point where most of it uses the
|
| |
+ same style.
|
| |
+
|
| |
+ Readability
|
| |
+ ^^^^^^^^^^^
|
| |
+
|
| |
+ Remember, code isn't just for compilers, it's for people, too. If it
|
| |
+ wasn't for people, we would all be programming in assembly. Coding style
|
| |
+ and consistency mean that if you go from one part of the code to another
|
| |
+ you don't spend time having to re-adjust from one style to another.
|
| |
+ Blocks are consistent and readable and the flow of the code is apparent.
|
| |
+ Coding style adds value for people who have to read your code after
|
| |
+ you've been hit by a bus. Remember that.
|
| |
+
|
| |
+ Definitions
|
| |
+ ^^^^^^^^^^^
|
| |
+
|
| |
+ The following guidelines apply to developing code in the SSSD project.
|
| |
+ Some rules are mandatory some rules are just suggestions or
|
| |
+ recommendations. We will use following terminology to identify whether
|
| |
+ this or that rule is mandatory or not.
|
| |
+
|
| |
+ - MUST – everybody must follow the rule
|
| |
+ - HIGHLY RECOMMENDED – the rule should be followed unless there are
|
| |
+ some serious reasons why they should not be followed.
|
| |
+ - RECOMMENDED – it is a best practice to use this rule. It is not
|
| |
+ required but strongly encouraged.
|
| |
+ - DISCRETION – follow this at developer discretion.
|
| |
+
|
| |
+ Note on C99
|
| |
+ -----------
|
| |
+
|
| |
+ We compile code with -std=gnu99 flag. It is OK to use C99 features
|
| |
+ supported by GCC and Clang, however C++ style line comments (*) should
|
| |
+ still be avoided. Using variable length arrays should be done with
|
| |
+ caution and only be used for smaller arrays.*
|
| |
+
|
| |
+ Rules
|
| |
+ -----
|
| |
+
|
| |
+ General Rules
|
| |
+ ^^^^^^^^^^^^^
|
| |
+
|
| |
+ - MUST: All source and header files must include a copy of the license.
|
| |
+ - HIGHLY RECOMMENDED: Stick to a K&R coding style for C.
|
| |
+ - HIGHLY RECOMMENDED: Curly braces on same line as if/while.
|
| |
+ - MUST: All code should be peer reviewed before being checked in.
|
| |
+ - MUST: If you are fixing a bug, add a link to the bug or upstream ticket
|
| |
+ to the commit message
|
| |
+ - HIGHLY RECOMMENDED: For Python we have an enforced style in many
|
| |
+ cases already but consistency is important.
|
| |
+
|
| |
+ Spaces and Indentation
|
| |
+ ^^^^^^^^^^^^^^^^^^^^^^
|
| |
+
|
| |
+ - MUST: No tabs all indentation 4 spaces.
|
| |
+ - HIGHLY RECOMMENDED: When wrapping lines, try to break it:
|
| |
+
|
| |
+ - after a comma
|
| |
+ - before an operator
|
| |
+ - align the new line with the beginnigng of the expression at the
|
| |
+ same level in the previous line
|
| |
+ - in case of long "if" statements, wrap the line before an operator
|
| |
+ and indent the new line
|
| |
+ - if all else fails, just indent 8 spaces.
|
| |
+
|
| |
+ Length of Line
|
| |
+ ^^^^^^^^^^^^^^
|
| |
+
|
| |
+ MUST: Do not make lines longer than 120 characters.
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Keep lines within 80 character boundary for better
|
| |
+ readability.
|
| |
+
|
| |
+ Comments
|
| |
+ ^^^^^^^^
|
| |
+
|
| |
+ MUST: Use only C style comments /\* \*/ not C++.
|
| |
+
|
| |
+ MUST: When commenting use the following styles: ::
|
| |
+
|
| |
+ /*
|
| |
+ * VERY important single-line comments look like this.
|
| |
+ */
|
| |
+
|
| |
+ /* Most single-line comments look like this. */
|
| |
+
|
| |
+ /*
|
| |
+ * Multi-line comments look like this. Make them real sentences. Fill
|
| |
+ * them so they look like real paragraphs.
|
| |
+ */
|
| |
+
|
| |
+ Avoid: ::
|
| |
+
|
| |
+ /* Multiline comments
|
| |
+ that look like this */
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Avoid useless comments that do not add value to the
|
| |
+ code.
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Each public API function should be preceded with a
|
| |
+ block comment describing what the function is supposed to do. It is not
|
| |
+ required to document internal APIs, although use your discretion and feel
|
| |
+ free to document internal functions as well.
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Block comments should be preceded by a blank line to
|
| |
+ set it apart. Line up the \* characters in the block comment.
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Python comments can use either the # or """ form
|
| |
+
|
| |
+ IFDEF
|
| |
+ ^^^^^
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: When using #ifdefs, it's nice to add comments in the
|
| |
+ pairing #endif: ::
|
| |
+
|
| |
+ #ifndef _HEADER_H_
|
| |
+ #define _HEADER_H_
|
| |
+
|
| |
+ /* something here */
|
| |
+
|
| |
+ #endif /* !_HEADER_H_ */
|
| |
+
|
| |
+ or: ::
|
| |
+
|
| |
+ #ifdef HAVE_PTHREADS
|
| |
+
|
| |
+ /* some code here */
|
| |
+
|
| |
+ #else /* !HAVE_PTHREADS */
|
| |
+
|
| |
+ /* some other code here */
|
| |
+
|
| |
+ #endif /* HAVE_PTHREADS */
|
| |
+
|
| |
+ Include Files
|
| |
+ ^^^^^^^^^^^^^
|
| |
+
|
| |
+ RECOMMENDED: Includes should be grouped properly. Standard headers and
|
| |
+ local headers should definitely be separated by a blank line. Other
|
| |
+ logical grouping should be reasonably done if needed. Files inside the
|
| |
+ groups should be sorted alphabetically, unless a specific order is
|
| |
+ required - this however is very rare, and must not happen. Also, one
|
| |
+ shouldn't depend on the fact that one header file includes other one,
|
| |
+ unless it is really obvious and/or desirable, like in cases when one
|
| |
+ header file practically "enhances" the other one, for example with more
|
| |
+ error codes, etc.
|
| |
+
|
| |
+ Macros
|
| |
+ ^^^^^^
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Macros that are unsafe should be in upper-case. This
|
| |
+ also applies to macros that span multiple lines: ::
|
| |
+
|
| |
+ #define MY_MACRO(a, b) do { \
|
| |
+ foo((a) + (b)); \
|
| |
+ bar(a); \
|
| |
+ } while (0)
|
| |
+
|
| |
+ Notice that arguments should be in parentheses if there's a risk. Also
|
| |
+ notice that a is referenced two times, and hence the macro is dangerous.
|
| |
+ Wrapping the body in do { } while (0) makes it safe to use it like this: ::
|
| |
+
|
| |
+ if (expr)
|
| |
+ MY_MACRO(x, y);
|
| |
+
|
| |
+ Notice the semicolon is used after the invocation, not in the macro
|
| |
+ definition.
|
| |
+
|
| |
+ Otherwise, if a macro is safe (for example a simple wrapping function),
|
| |
+ then the case can be lower-case.
|
| |
+
|
| |
+ Use of goto
|
| |
+ ^^^^^^^^^^^
|
| |
+
|
| |
+ We use goto to simplify cleanup operations and some other tasks that
|
| |
+ need to be done before leaving the function.
|
| |
+
|
| |
+ MUST: Never use goto to jump backwards in the code
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: If goto is needed in the code, use one of the
|
| |
+ following labels: done, fail, immediate.
|
| |
+
|
| |
+ RECOMMENDED: Do not use more than one goto label per function.
|
| |
+
|
| |
+ Label done
|
| |
+ """"""""""
|
| |
+
|
| |
+ Label done is used as jump target before exit. Clean-up operations, such
|
| |
+ as freeing local talloc context, usually follow the done label. Both
|
| |
+ successful and unsuccessful function executions pass this label.
|
| |
+
|
| |
+ Label fail
|
| |
+ """"""""""
|
| |
+
|
| |
+ Used as special exit path when function fails. Successful function
|
| |
+ execution typically does not execute statements after this label.
|
| |
+
|
| |
+ Label immediate
|
| |
+ """""""""""""""
|
| |
+
|
| |
+ The immediate label is used in tevent's _send functions. The typical usage
|
| |
+ would look like this: ::
|
| |
+
|
| |
+ if (ret != EOK) {
|
| |
+ DEBUG(...);
|
| |
+ goto immediate;
|
| |
+ }
|
| |
+
|
| |
+ immediate:
|
| |
+ if (ret == EOK) {
|
| |
+ tevent_req_done(req);
|
| |
+ } else {
|
| |
+ tevent_req_error(req, ret);
|
| |
+ }
|
| |
+ tevent_req_post(req, ev);
|
| |
+ return req;
|
| |
+
|
| |
+ Returning NULL from the tevent _send function should be reserved for OOM
|
| |
+ situations where the request cannot be created in the first place.
|
| |
+
|
| |
+ Variables
|
| |
+ ^^^^^^^^^
|
| |
+
|
| |
+ Naming
|
| |
+ """"""
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Use low case multi word underscore separated
|
| |
+ notation for naming variables.
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Make name meaningful.
|
| |
+
|
| |
+ MUST: Never use Hungarian notation when naming variables.
|
| |
+
|
| |
+ Declaring
|
| |
+ """""""""
|
| |
+
|
| |
+ Use ::
|
| |
+
|
| |
+ RECOMMENDED: One declaration per line is preferred.
|
| |
+ int foo;
|
| |
+ int bar;
|
| |
+
|
| |
+ instead of ::
|
| |
+
|
| |
+ int foo, bar;
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Initialize at declaration time when possible.
|
| |
+
|
| |
+ RECOMMENDED: Avoid complex variable initializations (like calling
|
| |
+ functions) when declaring variables like: ::
|
| |
+
|
| |
+ int foobar = get_foobar(baz);
|
| |
+
|
| |
+ but split it in: ::
|
| |
+
|
| |
+ int foobar;
|
| |
+
|
| |
+ foobar = get_foobar(baz);
|
| |
+ ...
|
| |
+
|
| |
+ MUST: Always declare variables at the top of the function or block. If
|
| |
+ you find yourself declaring many variables inside inner block or loop,
|
| |
+ consider refactoring the block into helper function. HIGHLY RECOMMENDED:
|
| |
+ Avoid shadowing variables. Use different name even if the shadowed
|
| |
+ variable is not important for the inner blocks.
|
| |
+
|
| |
+ RECOMMENDED: Don't initialize static or global variables to 0 or NULL.
|
| |
+
|
| |
+ Use of Typedefs
|
| |
+ """""""""""""""
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Avoid using typedefs. Typedefs obscure structures
|
| |
+ and make it harder to understand and debug.
|
| |
+
|
| |
+ Declaring Structures
|
| |
+ """"""""""""""""""""
|
| |
+
|
| |
+ DISCRETION: When defining structure or union try make it easy to read.
|
| |
+ You may use some form of alignment if you see that this might make it
|
| |
+ more readable.
|
| |
+
|
| |
+ Global Variables
|
| |
+ """"""""""""""""
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Avoid using global variables. They make for very
|
| |
+ poor code. Should be used only if no other way can be found. They tend
|
| |
+ to be not thread/async safe
|
| |
+
|
| |
+ Functions
|
| |
+ ^^^^^^^^^
|
| |
+
|
| |
+ External Function Declarations
|
| |
+ """"""""""""""""""""""""""""""
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Avoid situations where you have to explicitly list
|
| |
+ out external function. The header files should in general take care of
|
| |
+ the external function declaration. If this is not the case it is subject
|
| |
+ for review of the header file hierarchy.
|
| |
+
|
| |
+ Declaring Module Functions
|
| |
+ """"""""""""""""""""""""""
|
| |
+
|
| |
+ DISCRETION: It up to the developer to define the order of the functions
|
| |
+ in the module and thus declare functions at the top or use a native flow
|
| |
+ of the module and avoid forward function declarations.
|
| |
+
|
| |
+ Order of the Functions
|
| |
+ """"""""""""""""""""""
|
| |
+
|
| |
+ DISCRETION: It is up to the developer which approach to use: whether to
|
| |
+ write the main function at the top of the module and then all the
|
| |
+ supporting functions or start with supporting functions and have the
|
| |
+ main one at the bottom. Both approaches are acceptable. One can use
|
| |
+ additional comments to help identify how the module is structured.
|
| |
+
|
| |
+ Naming Functions
|
| |
+ """"""""""""""""
|
| |
+
|
| |
+ MUST: For function names use multi word underscore separate naming
|
| |
+ convention like this monitor\_task\_init(struct task\_server \*task);
|
| |
+
|
| |
+ MUST: Never use Hungarian notation when naming functions.
|
| |
+
|
| |
+ Indenting Functions
|
| |
+ """""""""""""""""""
|
| |
+
|
| |
+ DISCRETION: It is up to the developer which pattern to use when
|
| |
+ indenting the function parameters if function has long name and has to
|
| |
+ be split between multiple lines. The pattern however MUST be consistent
|
| |
+ across the module so if you are fixing somebodies code continue with the
|
| |
+ pattern used in the module.
|
| |
+
|
| |
+ Function Declaration
|
| |
+ """"""""""""""""""""
|
| |
+
|
| |
+ DISCRETION: It is up to the developer whether to put the return type of
|
| |
+ the function and modifiers (static for example) in front of the function
|
| |
+ on the same line or start the line with the an actual function name. In
|
| |
+ any case the pattern MUST be consistent across the module. If you are
|
| |
+ adding function to an already existing module follow its pattern. MUST:
|
| |
+ Put opening “{“ of the function body on the beginning of the new line
|
| |
+ after the function declaration. HIGHLY RECOMMENDED: Do not put spaces
|
| |
+ before or after parenthesis in the declaration of the parameters. For
|
| |
+ example:
|
| |
+
|
| |
+ OK: ::
|
| |
+ int foo(int bar, int baz);
|
| |
+
|
| |
+ NOT OK: ::
|
| |
+ int bad ( arg1 , arg2 );
|
| |
+
|
| |
+ Function Parameters
|
| |
+ """""""""""""""""""
|
| |
+
|
| |
+ RECOMMENDED: Try to always put "input" arguments before "output"
|
| |
+ arguments, if you have arguments that provide both input an output put
|
| |
+ them between the pure-input and the pure-output ones. Add underscore
|
| |
+ prefix "\_" to output arguments.
|
| |
+
|
| |
+ OK: ::
|
| |
+
|
| |
+ foo(int in1, void *in2, char **_ou1);
|
| |
+
|
| |
+ NOT OK: ::
|
| |
+
|
| |
+ voo(char **ou1, int in1);
|
| |
+
|
| |
+ Use of Const
|
| |
+ """"""""""""
|
| |
+
|
| |
+ RECOMMENDED: If appropriate, always use the const modifier for pointers
|
| |
+ passed to the function. This makes the intentions of the function more
|
| |
+ clearer, plus allows the compiler to catch more bugs and make some
|
| |
+ optimizations.
|
| |
+
|
| |
+ Tools to Use
|
| |
+ """"""""""""
|
| |
+
|
| |
+ RECOMMENDED: Creating lists and queues was already done a lot of times.
|
| |
+ When possible, use some common functions for manipulating these to avoid
|
| |
+ mistakes.
|
| |
+
|
| |
+ Conditions and Statements
|
| |
+ ^^^^^^^^^^^^^^^^^^^^^^^^^
|
| |
+
|
| |
+ Condition
|
| |
+ """""""""
|
| |
+
|
| |
+ RECOMMENDED: Use the full condition syntax like (str == NULL) rather
|
| |
+ than (!str).
|
| |
+
|
| |
+ IF Statements
|
| |
+ """""""""""""
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: If-else statements should have the following form: ::
|
| |
+
|
| |
+ if (''condition'') {
|
| |
+ /* do some work */
|
| |
+ }
|
| |
+
|
| |
+ if (''condition'') {
|
| |
+ /* do some work */
|
| |
+ } else {
|
| |
+ /* do some other work */
|
| |
+ }
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Balance the braces in the if and else in an if-else
|
| |
+ statement if either has only one line: ::
|
| |
+
|
| |
+ if (condition) {
|
| |
+ /*
|
| |
+ * stuff that takes up more than one
|
| |
+ * line
|
| |
+ */
|
| |
+ } else {
|
| |
+ /* stuff that only uses one line */
|
| |
+ }
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Use braces even if there s just one line in the if
|
| |
+ statement.
|
| |
+
|
| |
+ DISCRETION: You can avoid the braces if entire if statement is on one
|
| |
+ line.
|
| |
+
|
| |
+ NOT OK: ::
|
| |
+
|
| |
+ if (foo)
|
| |
+ bar();
|
| |
+
|
| |
+ OK: ::
|
| |
+
|
| |
+ if (foo) {
|
| |
+ bar();
|
| |
+ }
|
| |
+
|
| |
+ Also OK: ::
|
| |
+
|
| |
+ if (foo) bar();
|
| |
+
|
| |
+ Always use braces if there is an else part: ::
|
| |
+
|
| |
+ if (foo) {
|
| |
+ bar();
|
| |
+ } else {
|
| |
+ baz();
|
| |
+ }
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Avoid last-return-in-else problem. Code should look
|
| |
+ like this: ::
|
| |
+
|
| |
+ int foo(int bar)
|
| |
+ {
|
| |
+ if (something) {
|
| |
+ /* stuff done here */
|
| |
+ return 1;
|
| |
+ }
|
| |
+
|
| |
+ return 0;
|
| |
+ }
|
| |
+
|
| |
+ **NOT** like this: ::
|
| |
+
|
| |
+ int foo(int bar)
|
| |
+ {
|
| |
+ if (something) {
|
| |
+ /* stuff done here */
|
| |
+ return 1;
|
| |
+ } else {
|
| |
+ return 0;
|
| |
+ }
|
| |
+ }
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Conditions with <, <=, >= or == operators should
|
| |
+ isolate the value being checked (untrusted value) on the left hand side
|
| |
+ of the comparison. The right hand side should contain trusted values
|
| |
+ (thus avoiding overflows/underflows). Use unsigned types when storing
|
| |
+ sizes or lengths. For example if variable len is untrusted and variables
|
| |
+ size and p are trusted (NOTE: sizes and length should be stored in
|
| |
+ unsigned types):
|
| |
+
|
| |
+ OK: ::
|
| |
+ if (len > size - p) return EINVAL;
|
| |
+
|
| |
+ NOT OK: ::
|
| |
+ if ((p + len ) > size) return EINVAL;
|
| |
+
|
| |
+ Loops
|
| |
+ """""
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: For, while and until statements should take a
|
| |
+ similar form: ::
|
| |
+
|
| |
+ for (''initialization; condition; update'') {
|
| |
+ /* iterate here */
|
| |
+ }
|
| |
+
|
| |
+ while (''condition'') {
|
| |
+ /* do some work */
|
| |
+ }
|
| |
+
|
| |
+ Switch
|
| |
+ """"""
|
| |
+
|
| |
+ HIGHLY RECOMMENDED: Use the following style for the switch statements.
|
| |
+ Add comments if missing break is intentional. ::
|
| |
+
|
| |
+ switch (var) {
|
| |
+ case 0:
|
| |
+ break;
|
| |
+ case 1:
|
| |
+ printf("meh.\n");
|
| |
+ /* FALLTHROUGH */
|
| |
+ case 2:
|
| |
+ printf("2\n");
|
| |
+ break;
|
| |
+ default:
|
| |
+ /* Always have default */
|
| |
+ break;
|
| |
+ }
|
| |
+
|
| |
+ Strings
|
| |
+ ^^^^^^^
|
| |
+
|
| |
+ Internationalized (i18n) Strings
|
| |
+ """"""""""""""""""""""""""""""""
|
| |
+
|
| |
+ If the string will be internationalized (e.g. is marked with \_()) and
|
| |
+ it has more than one format substitution you ***MUST*** use *index*
|
| |
+ format specifiers, not positional format specifiers. Translators need
|
| |
+ the option to reorder where substitutions appear in a string because the
|
| |
+ ordering of nouns, verbs, phrases, etc. differ between languages. If
|
| |
+ conventional positional format conversion specifiers (e.g. %s %d) are
|
| |
+ used the string cannot be reordered because the ordering of the format
|
| |
+ specifiers must match the ordering of the printf arguments supplying the
|
| |
+ substitutions. The fix for this is easy, use indexed format specifiers.
|
| |
+ An indexed specifier includes an (1 based) index to the % character that
|
| |
+ introduces the format specifier (e.g. %1$ to indicate the first
|
| |
+ argument). That index is used to select the matching argument from the
|
| |
+ argument list. When indexed specifiers are used *all* format specifiers
|
| |
+ and *all* \* width fields ***MUST*** use indexed specifiers.
|
| |
+
|
| |
+ Here is an example of incorrect usage with positional specifiers: ::
|
| |
+
|
| |
+ printf(_("item %s has %s value"), name, value);
|
| |
+
|
| |
+ Here is the correct usage using indexed specifiers: ::
|
| |
+
|
| |
+ printf(_("item %1$s has %2$s value"), name, value);
|
| |
+
|
| |
+ See man 3 printf as well as section 15.3.1 "C Format Strings" in the GNU
|
| |
+ gettext manual for more details.
|
| |
fidencio
commented 6 years ago
This is pretty much a copy & paste from the original one, with basically
a few changes:
repo;
commit template.
Signed-off-by: Fabiano Fidêncio fidencio@redhat.com