Changes between Initial Version and Version 1 of BIND9CodingGuidelines


Ignore:
Timestamp:
Aug 31, 2009, 2:23:32 PM (8 years ago)
Author:
shane
Comment:

--

Legend:

Unmodified
Added
Removed
Modified
  • BIND9CodingGuidelines

    v1 v1  
     1''These guidelines are for BIND 9, and are presented here for discussion and reference only.''
     2
     3= C Language =
     4An ANSI standard C compiler and library are assumed. Feel free to use any ANSI C feature.
     5
     6= Warnings =
     7Given a reasonable set of things to warn about (e.g. -W -Wall for gcc), the goal is to compile with no warnings.
     8
     9= C Source Code =
     10== Copyright ==
     11All source files should have a copyright. The copyright year(s) should be kept current. The files and the copyright year(s) should be listed in util/copyrights.
     12
     13== Line Formatting ==
     14=== Indentation ===
     15Use tabs. Spaces are only allowed when needed to line up a continued expression. In the following example, spaces used for indentation are indicated with "_":
     16
     17{{{
     18        printf("this is going to be %s very long %s statement\n",
     19        _______"a", "printf");
     20}}}
     21
     22=== Vertical Whitespace ===
     23Vertical whitespace is also encouraged for improved code legibility by grouping closely related statements and then separating them with a single empty line. There should not, however, be more than one empty adjacent line anywhere.
     24
     25=== Line Length ===
     26Lines should not be longer than 79 characters, even if it requires violating the indentation rules to do so. Since ANSI is assumed, the best way to deal with strings that extend past column 79 is to break them into two or more sections separated from each other by a newline and indentation:
     27
     28{{{
     29                                  puts("This string got very far to the "
     30                                       "left and wrapped.  ANSI catenation "
     31                                       "rules will turn this into one "
     32                                       "long string.");
     33}}}
     34
     35== Comments ==
     36Comments should be used anytime they improve the readability of the code.
     37
     38Comments may be single-line or multiline. A single-line comment should be at the end of the line if there is other text on the line, and should start in the same column as other nearby end-of-line comments. The comment should be at the same indentation level as the text it is referring to. Multiline comments should start with "/*" on a line by itself. Subsequent lines should have " *" lined-up with the "*" above. The end of the comment should be " */" on a line by itself, again with the "*" lined-up with the one above. Comments should start with a capital letter and end with a period.
     39
     40Good:
     41
     42{{{
     43        /*
     44         * Private variables.
     45         */
     46
     47        static int              a               /* Description of 'a'. */
     48        static int              b               /* Description of 'b'. */
     49        static char *           c               /* Description of 'c'. */
     50}}}
     51The following lint and lint-like comments should be used where appropriate:
     52
     53{{{
     54        /* ARGSUSED */
     55        /* FALLTHROUGH */
     56        /* NOTREACHED */
     57        /* VARARGS */
     58}}}
     59
     60== .h files ==
     61.h files should not rely on other files having been included. .h files should prevent multiple inclusion. The OS is assumed to prevent multiple inclusion of its .h files.
     62
     63.h files that define modules should have a structure like the following. Note that should be included by any public header file to get the ISC_LANG_BEGINDECLS and ISC_LANG_ENDDECLS macros used so the correct name-mangling happens for function declarations when C++ programs include the file. should be included for private header files or for public files that do not declare any functions.
     64
     65{{{
     66/*
     67 * Copyright (C) 1998  Internet Software Consortium.
     68 *
     69 * Permission to use, copy, modify, and distribute this software for any
     70 * purpose with or without fee is hereby granted, provided that the above
     71 * copyright notice and this permission notice appear in all copies.
     72 *
     73 * THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SOFTWARE CONSORTIUM DISCLAIMS
     74 * ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES
     75 * OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL INTERNET SOFTWARE
     76 * CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL
     77 * DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR
     78 * PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS
     79 * ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS
     80 * SOFTWARE.
     81 */
     82
     83#ifndef ISC_WHATEVER_H
     84#define ISC_WHATEVER_H 1
     85
     86/*****
     87 ***** Module Info
     88 *****/
     89
     90/*
     91 * (Module name here.)
     92 *
     93 * (One line description here.)
     94 *
     95 * (Extended description and notes here.)
     96 *
     97 * MP:
     98 *      (Information about multiprocessing considerations here, e.g. locking
     99 *       requirements.)
     100 *
     101 * Reliability:
     102 *      (Any reliability concerns should be mentioned here.)
     103 *
     104 * Resources:
     105 *      (A rough guide to how resources are used by this module.)
     106 *
     107 * Security:
     108 *      (Any security issues are discussed here.)
     109 *
     110 * Standards:
     111 *      (Any standards relevant to the module are listed here.)
     112 */
     113
     114/***
     115 *** Imports
     116 ***/
     117
     118/* #includes here. */
     119#include <isc/lang.h>
     120
     121/***
     122 *** Types
     123 ***/
     124
     125/* (Type definitions here.) */
     126
     127/***
     128 *** Functions
     129 ***/
     130ISC_LANG_BEGINDECLS
     131/* (Function declarations here, with full prototypes.) */
     132ISC_LANG_ENDDECLS
     133
     134#endif /* ISC_WHATEVER_H */
     135}}}
     136
     137== C Source ==
     138=== Including Interfaces (.h files) ===
     139The first file to be included in a C source file must be config.h. The config.h file must never be included by any public header file (that is, any header file that will be installed by "make install"). Try to include only necessary files, not everything under the sun.
     140
     141Operating-system-specific files should not be included by most modules.
     142
     143Include UNIX "sys" .h files before ordinary C includes.
     144
     145=== Statements ===
     146There should be at most one statement per line. The comma operator should not be used to form compound statements.
     147
     148Bad:
     149
     150{{{
     151        if (i > 0) {
     152                printf("yes\n"); i = 0; j = 0;
     153                x = 4, y *= 2;
     154        }
     155}}}
     156
     157=== Functions ===
     158The use of ANSI C function prototypes is required.
     159
     160The return type of the function should be listed on a line by itself when specifying the implementation of the function. The opening curly brace should occur on the same line as the argument list, unless the argument list is more than one line long.
     161
     162Good:
     163
     164{{{
     165static inline void
     166f(int i) {
     167        /* whatever */
     168}
     169
     170int
     171g(int i, /* other args here */
     172  int last_argument)
     173{
     174        return (i * i);
     175}
     176}}}
     177
     178To suppress compiler warnings, unused function arguments are declared using the UNUSED() macro.
     179
     180In the function body, local variable declarations are followed by any REQUIRE()s, UNUSED() declarations, and other code, in this order. These sections are separated by blank lines.
     181
     182=== Curly Braces ===
     183Curly Braces do not get their own indentation. An opening brace does not start a new line. The statements enclosed by the braces should not be on the same line as the opening or closing brace. A closing brace should be the only thing on the line, unless it's part of an else clause.
     184
     185Generally speaking, when a control statement (if, for or while) has only a single action associated with it, then no bracing is used around the statement. Exceptions include when the compiler would complain about an ambiguous else clause, or when extra bracing improves the readability (a judgement call biased toward not having the braces).
     186
     187Good:
     188
     189{{{
     190static void
     191f(int i) {
     192        if (i > 0) {
     193                printf("yes\n");
     194                i = 0;
     195        } else
     196                printf("no\n");
     197}
     198}}}
     199Bad:
     200
     201{{{
     202void f(int i)
     203  {
     204    if(i<0){i=0;printf("was negative\n");}
     205    if (i > 0)
     206      {
     207        printf("yes\n");
     208        i = 0;
     209      }}
     210}}}
     211
     212=== Spaces ===
     213
     214    * Do put a space between operators like '=', '+', '==', etc.
     215    * Do put a space after ','.
     216    * Do put a space after ';' in a 'for' statement.
     217    * Do put a space after 'return', and also parenthesize the return value.
     218
     219    * Do not put a space between a variable or function name and '(' or '['.
     220    * Do not put a space after the "sizeof" operator name, and also parenthesize its argument, as in malloc(4 * sizeof(long)).
     221    * Do not put a space immediately after a '(' or immediately before a ')', unless it improves readability. The same goes for '[' and ']'.
     222    * Do not put a space before '++' or '--' when used in post-increment/decrement mode, or after them when used in pre-increment/decrement mode.
     223    * Do not put a space before ';' when terminating a statement or in a 'for' statement.
     224    * Do not put a space after '*' when used to dereference a pointer, or on either side of '->'.
     225    * Do not put a space after '~'.
     226    * The '|' operator may either have a space on both sides or it may have no spaces.
     227    * Do not put a space after a cast.
     228
     229=== Return Values ===
     230If a function returns a value, it should be cast to (void) if you don't care what the value is, except for printf and its variants, fputc, fwrite (when writing text), fflush, memcpy, memmove, memset, strcpy, strncpy, and strcat.
     231
     232Certain functions return values or not depending on the operating system or even compiler flags; these include these include openlog and srandom. The return value of these should not be used nor cast to (void).
     233
     234All error conditions must be handled.
     235
     236Mixing of error status and valid results within a single type should be avoided.
     237
     238Good:
     239
     240{{{
     241        os_descriptor_t         s;
     242        os_result_t             result;
     243
     244        result = os_socket_create(AF_INET, SOCK_STREAM, 0, &s);
     245        if (result != OS_R_SUCCESS) {
     246                /* Do something about the error. */
     247                return;
     248        }
     249}}}
     250Not so good:
     251
     252{{{
     253        int s;
     254
     255        /*
     256         * Obviously using interfaces like socket() (below) is allowed
     257         * since otherwise you couldn't call operating system routines; the
     258         * point is not to write more interfaces like them.
     259         */
     260        s = socket(AF_INET, SOCK_STREAM, 0);
     261        if (s < 0) {
     262                /* Do something about the error using errno. */
     263                return;
     264        }
     265}}}
     266=== Integral Types ===
     267Careful thought should be given to whether an integral type should be signed or unsigned, and to whether a specific size is required. "int" should be used for generic variables (e.g. iteration counters, array subscripts). Other than for generic variables, if a negative value isn't meaningful, the variable should be unsigned. Assignments and comparisons between signed and unsigned integers should be avoided; suppressing the warnings with casts is not desireable.
     268
     269=== Casting ===
     270Casting should be avoided when possible. When it is necessary, there should be no space between the cast and what is being cast.
     271
     272Bad (obviously for more than one reason ...):
     273
     274{{{
     275        (void) malloc(SMBUF);
     276}}}
     277
     278=== Clear Success or Failure ===
     279A function should report success or failure, and do so accurately. It should never fail silently. Use of Design by Contract can help here.
     280
     281When a function is designed to return results to the caller by assigning to caller variables through pointer arguments, it should perform the assignment only if it succeeds and leave the variables unmodified if it fails.
     282
     283=== Testing Bits ===
     284Bit testing should be as follows:
     285
     286Good:
     287
     288{{{
     289        /* Test if flag set. */
     290        if ((flags & FOO) != 0) {
     291
     292        }
     293        /* Test if flag clear. */
     294        if ((flags & BAR) == 0) {
     295
     296        }
     297        /* Test if both flags set. */
     298        if ((flags & (FOO|BAR)) == (FOO|BAR)) {
     299
     300        }
     301}}}
     302Bad:
     303
     304{{{
     305        /* Test if flag set. */
     306        if (flags & FOO) {
     307
     308        }
     309        /* Test if flag clear. */
     310        if (! (flags & BAR)) {
     311
     312        }
     313}}}
     314
     315=== Pointers ===
     316==== Null Pointer ====
     317The null pointer value should be referred to with "NULL", not with "0". Testing to see whether a pointer is NULL should be explicit.
     318
     319Good:
     320
     321{{{
     322        char *c = NULL;
     323
     324        /* ... */
     325
     326        if (c == NULL) {
     327                /* Do something. */
     328        }
     329}}}
     330
     331==== Invalidating Pointers ====
     332When the data a pointer points to has been freed, or is otherwise no longer valid, the pointer should be set to NULL unless the pointer is part of a structure which is itself going to be freed immediately.
     333
     334Good:
     335
     336{{{
     337        char *text;
     338
     339        /* text is initialized here. */
     340
     341        free(text);
     342        text = NULL;
     343}}}
     344
     345=== Testing for Zero or Non-zero ===
     346Explicit testing against zero is required for numeric, non-boolean variables.
     347
     348Good:
     349
     350{{{
     351        int i = 10;
     352
     353        /* ... */
     354
     355        if (i != 0) {
     356                /* Do something. */
     357        }
     358}}}
     359
     360Bad:
     361
     362{{{
     363        int i = 10;
     364
     365        /* ... */
     366
     367        if (i) {
     368                /* Do something. */
     369        }
     370}}}
     371
     372=== The Ternary Operator ===
     373The ?: operator should mostly be avoided. It is tolerated when deciding what value to pass as a parameter to a function, such as frequently happens with printf, and also when a simple (non-compound) value is being used in assignment or as part of a calculation. In particular, using the ternary operator to specify a return value is verboten.
     374
     375Good:
     376
     377{{{
     378        printf("%c is%s a number.\n", c, isdigit(c) ? "" " NOT");
     379        l = (l1 < l2) ? l1 : l2;
     380        if (gp.length + (go < 16384 ? 2 : 3) >= name->length) {
     381           ...
     382        }
     383}}}
     384
     385Bad:
     386
     387{{{
     388        return (success ? ISC_R_SUCESS : ISC_R_FAILURE);
     389}}}
     390
     391=== Assignment in Parameters ===
     392Variables should not have their values assigned or changed when being passed as parameters, except perhaps for the increment and decrement operators.
     393
     394Bad:
     395
     396{{{
     397        malloc(size = 20);
     398}}}
     399
     400Ok:
     401
     402{{{
     403        fputc(c++, stdout);
     404}}}
     405
     406== Namespace ==
     407=== Public Interfaces ===
     408All public interfaces to functions, macros, typedefs, and variables provided by the library, should use names of the form {library}_{module}_{what}, such as:
     409
     410{{{
     411        isc_buffer_t                            /* typedef */
     412        dns_name_setbuffer(name, buffer)        /* function */
     413        ISC_LIST_HEAD(list)                     /* macro */
     414        isc_commandline_argument                /* variable */
     415}}}
     416however, structures which are typedef'd generally have the name of the typedef sans the final _t:
     417
     418{{{
     419        struct dns_rbtnode {
     420                /* ... members ... */
     421        }
     422}}}
     423Generally speaking macros are defined with all capital letters, but this is not universally consistent (eg, numerous isc_buffer_{foo} macros).
     424
     425The {module} and {what} segments of the name do not have underscores separating natural word elements, as demonstrated in isc_commandline_argument and dns_name_setbuffer above. The {module} part is usually the same as the basename of the source file, but sometimes other {module} interfaces appear within one file, such as dns_label_* interfaces in lib/dns/name.c. However, in the public libraries the file name must be the same as some module interface provided by the file; e.g., dns_rbt_* interfaces would not be declared in a file named redblack.c (in lieu of any other dns_redblack_* interfaces in the file).
     426
     427The one notable exception to this naming rule is the interfaces provided by . There's a large caveat associated with the public description of this file that it is hazardous to use because it pollutes the general namespace.
     428
     429=== Shared Private Interfaces ===
     430When a module provides an interface for internal use by other modules in the library, it should use the same naming convention described for the public interfaces, except {library} and {module} are separated by a double-underscore. This indicates that the name is internal, its API is not as formal as the public API, and thus it might change without any sort of notice.
     431
     432== Initialization ==
     433When an object is allocated from the heap, all fields in the object must be initialized.
     434
     435== Dead Code Pruning ==
     436Source which becomes obsolete should be removed, not just disabled with #if 0 ... #endif.
     437
     438== Log messages ==
     439Error and warning messages should be logged through the logging system. Debugging printfs may be used during development, but must be removed when the debugging is finished. The UNEXPECTED_ERROR() macro is obsolete and should not be used in new code.
     440
     441Log messages do not start with a capital letter, nor do they end in a period.
     442
     443When variable text such as a file name or domain name occurs as part of an English phrase, it should be enclosed in single quotes, as in "zone '%s' is lame".
     444
     445When the variable text forms a separate phrase, such as when it separated from the rest of the message by a colon, it can be left unquoted. E.g., isc_log_write(... "open: %s: %s", filename, isc_result_totext(result));
     446
     447Function names, line numbers, memory addresses, and other references to program internals may be used in debugging messages and in messages to report programming errors detected at runtime. They may not be used in messages that indicate errors in the program's inputs or operation.
     448
     449= Perl source code =
     450Perl must not be required for building, installing, or using the BIND 9 name server. It may be used for things like test scripts and optional server add-on components.
     451
     452Perl 5 is assumed; Perl scripts do not need to work in Perl 4.
     453
     454Perl source code should follow the conventions for C source code where applicable.
     455