coding.html revision 7e985260692958285fc3c128fdb27867a865db26
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync<H2>C Language</H2>
6dd8f5023a9ba7588212331db90059553136fe33vboxsync
99be02f9e15a3ca61b6a7c207cc7eb68dbd04817vboxsyncAn ANSI standard C compiler and library are assumed. Feel free to use any
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncANSI C feature.<P>
99be02f9e15a3ca61b6a7c207cc7eb68dbd04817vboxsync
99be02f9e15a3ca61b6a7c207cc7eb68dbd04817vboxsync<H2>Warnings</H2>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncGiven a reasonable set of things to warn about (e.g. -W -Wall for gcc), the
e64031e20c39650a7bc902a3e1aba613b9415deevboxsyncgoal is to compile with no warnings.
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync<H2>C Source Code</H2>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync<H3>Copyright</H3>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncAll source files should have a copyright. The copyright year(s)
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncshould be kept current. The files and the copyright year(s) should be
19a258565a4d24e5a0af62f626943ac9b898d957vboxsynclisted in util/copyrights.<P>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync<H3>Line Formatting</H3>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync<H4>Indentation</H4>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncUse tabs. Spaces are only allowed when needed to line up a continued
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncexpression. In the following example, spaces used for indentation are
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncindicated with "_":
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync<PRE><CODE>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync printf("this is going to be %s very long %s statement\n",
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync _______"a", "printf");
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync</CODE></PRE>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync<H4>Vertical Whitespace</H4>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncVertical whitespace is also encouraged for improved code legibility by
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncgrouping closely related statements and then separating them with a
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncsingle empty line. There should not, however, be more than one empty
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncadjacent line anywhere.
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync<H4>Line Length</H4>
dbc4cb7b07245195230a32d42f56d7edf888fcadvboxsyncLines should not be longer than 79 characters, even if it requires
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncviolating the indentation rules to do so. Since ANSI is assumed, the
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncbest way to deal with strings that extend past column 79 is to break
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncthem into two or more sections separated from each other by a newline
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncand indentation:
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync<PRE><CODE>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync puts("This string got very far to the "
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync "left and wrapped. ANSI catenation "
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync "rules will turn this into one
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync "long string.");
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync</CODE></PRE>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync<H3>Comments</H3>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncComments should be used anytime they improve the readability of the code.<P>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncComments may be single-line or multiline. A single-line comment should be
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncat the end of the line of there is other text on the line, and should start
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncin the same column as other nearby end-of-line comments. The comment
6dd8f5023a9ba7588212331db90059553136fe33vboxsyncshould be at the same indentation level as the text it is referring to.
dbc4cb7b07245195230a32d42f56d7edf888fcadvboxsyncMultiline comments should start with "/*" on a line by itself. Subsequent
dbc4cb7b07245195230a32d42f56d7edf888fcadvboxsynclines should have " *" lined-up with the "*" above. The end of the comment
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncshould be " */" on a line by itself, again with the "*" lined-up with the
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncone above. Comments should start with a capital letter and end with a
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncperiod.<P>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncGood:<P>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync<PRE><CODE>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync /*
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync * Private variables.
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync */
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync static int a /* Description of 'a'. */
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync static int b /* Description of 'b'. */
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync static char * c /* Description of 'c'. */
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync</CODE></PRE>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync
19a258565a4d24e5a0af62f626943ac9b898d957vboxsyncThe following lint and lint-like comments should be used where appropriate:
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync<PRE><CODE>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync /* ARGSUSED */
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync /* FALLTHROUGH */
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync /* NOTREACHED */
e43535ace2499e7f8ef8822186047979bd58d464vboxsync /* VARARGS */
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync</CODE></PRE>
19a258565a4d24e5a0af62f626943ac9b898d957vboxsync
<H3>.h files</H3>
.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.<P>
.h files that define modules should have a structure like the
following. Note that <isc/lang.h> 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. <isc/lang.h> should
be included for private header files or for public files that do not
declare any functions.<P>
<PRE><CODE>
/*
* Copyright (C) 1998 Internet Software Consortium.
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
* copyright notice and this permission notice appear in all copies.
*
* THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SOFTWARE CONSORTIUM DISCLAIMS
* ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES
* OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL INTERNET SOFTWARE
* CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL
* DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR
* PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS
* ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS
* SOFTWARE.
*/
#ifndef ISC_WHATEVER_H
#define ISC_WHATEVER_H 1
/*****
***** Module Info
*****/
/*
* (Module name here.)
*
* (One line description here.)
*
* (Extended description and notes here.)
*
* MP:
* (Information about multiprocessing considerations here, e.g. locking
* requirements.)
*
* Reliability:
* (Any reliability concerns should be mentioned here.)
*
* Resources:
* (A rough guide to how resources are used by this module.)
*
* Security:
* (Any security issues are discussed here.)
*
* Standards:
* (Any standards relevant to the module are listed here.)
*/
/***
*** Imports
***/
/* #includes here. */
#include &lt;isc/lang.h&gt;
/***
*** Types
***/
/* (Type definitions here.) */
/***
*** Functions
***/
ISC_LANG_BEGINDECLS
/* (Function declarations here, with full prototypes.) */
ISC_LANG_ENDDECLS
#endif /* ISC_WHATEVER_H */
</CODE></PRE>
<H3>C Source</H3>
<H4>Including Interfaces (.h files)</H4>
The 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.<P>
Operating-system-specific files should not be included by most modules.<P>
Include UNIX "sys" .h files before ordinary C includes.<P>
<H4>Statements</H4>
There should be at most one statement per line. The comma operator
should not be used to form compound statements.<P>
Bad:<P>
<PRE><CODE>
if (i > 0) {
printf("yes\n"); i = 0; j = 0;
x = 4, y *= 2;
}
</CODE></PRE>
<H4>Functions</H4>
The use of ANSI C function prototypes is required.<P>
The 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.<P>
Good:<P>
<PRE><CODE>
static inline void
f(int i) {
/* whatever */
}
int
g(int i, /* other args here */
int last_argument)
{
return (i * i);
}
</CODE></PRE>
To suppress compiler warnings, unused function arguments are declared
using the <CODE>UNUSED()</CODE> macro.<P>
In the function body, any <CODE>REQUIRE()</CODE>s are placed
immediately after the local variabled, followed by any
<CODE>UNUSED()</CODE> declarations.<P>
<H4>Curly Braces</H4>
Curly 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.<P>
Generally speaking, when a control statement (<CODE>if, for</CODE> or
<CODE>while</CODE>) 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).<P>
Good:<P>
<PRE><CODE>
static void
f(int i) {
if (i > 0) {
printf("yes\n");
i = 0;
} else
printf("no\n");
}
</CODE></PRE>
Bad:<P>
<PRE><CODE>
void f(int i)
{
if(i<0){i=0;printf("was negative\n");}
if (i > 0)
{
printf("yes\n");
i = 0;
}}
</CODE></PRE>
<H4>Spaces</H4>
<UL>
<LI>Do put a space between operators like '=', '+', '==', etc.
<LI>Do put a space after ','.
<LI>Do put a space after ';' in a 'for' statement.
<LI>Do put a space after 'return', and also parenthesize the return value.
</UL>
<UL>
<LI>Do not put a space between a variable or function name and '(' or '['.
<LI>Do not put a space after the "sizeof" operator name, and also
parenthesize its argument, as in <CODE>malloc(4 * sizeof(long))</CODE>.
<LI>Do not put a space immediately after a '(' or immediately before a ')',
unless it improves readability. The same goes for '[' and ']'.
<LI>Do not put a space before '++' or '--' when used in
post-increment/decrement mode, or after them when used in
pre-increment/decrement mode.
<LI>Do not put a space before ';' when terminating a statement or in a 'for'
statement.
<LI>Do not put a space after '*' when used to dereference a pointer, or on
either side of '->'.
<LI>Do not put a space after '~'.
<LI>The '|' operator may either have a space on both sides or it may have no
spaces.
<LI>Do not put a space after a cast.
</UL>
<H4>Return Values</H4>
If a function returns a value, it should be cast to (void) if you don't
care what the value is, except for <CODE>printf</CODE><P>
All error conditions must be handled.<P>
Mixing of error status and valid results within a single type should be
avoided.<P>
Good:
<PRE><CODE>
os_descriptor_t s;
os_result_t result;
result = os_socket_create(AF_INET, SOCK_STREAM, 0, &s);
if (result != OS_R_SUCCESS) {
/* Do something about the error. */
return;
}
</CODE></PRE>
Not so good:
<PRE><CODE>
int s;
/*
* Obviously using interfaces like socket() (below) is allowed
* since otherwise you couldn't call operating system routines; the
* point is not to write more interfaces like them.
*/
s = socket(AF_INET, SOCK_STREAM, 0);
if (s < 0) {
/* Do something about the error using errno. */
return;
}
</CODE></PRE>
<H4>Integral Types</H4>
Careful 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.<P>
<H4>Casting</H4>
Casting should be avoided when possible. When it is necessary, there
should be no space between the cast and what is being cast.<P>
Bad (obviously for more than one reason ...):
<PRE><CODE>
(void) malloc(SMBUF);
</CODE></PRE>
<H4>Clear Success or Failure</H4>
A function should report success or failure, and do so accurately. It
should never fail silently. Use of Design by Contract can help here.<P>
<H4>Testing Bits</H4>
Bit testing should be as follows:<P>
Good:
<PRE><CODE>
/* Test if flag set. */
if ((flags & FOO) != 0) {
}
/* Test if flag clear. */
if ((flags & BAR) == 0) {
}
/* Test if both flags set. */
if ((flags & (FOO|BAR)) == (FOO|BAR)) {
}
</CODE></PRE>
Bad:
<PRE><CODE>
/* Test if flag set. */
if (flags & FOO) {
}
/* Test if flag clear. */
if (! (flags & BAR)) {
}
</CODE></PRE>
<H4>Pointers</H4>
<H5>Null Pointer</H5>
The null pointer value should be referred to with "NULL", not with "0".
Testing to see whether a pointer is NULL should be explicit.<P>
Good:
<PRE><CODE>
char *c = NULL;
/* ... */
if (c == NULL) {
/* Do something. */
}
</CODE></PRE>
<H5>Invalidating Pointers</H5>
When 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.<P>
Good:
<PRE><CODE>
char *text;
/* text is initalized here. */
free(text);
text = NULL;
</CODE></PRE>
<H4>Testing for Zero or Non-zero</H4>
Explicit testing against zero is required for numeric, non-boolean variables.
<P>
Good:
<PRE><CODE>
int i = 10;
/* ... */
if (i != 0) {
/* Do something. */
}
</CODE></PRE>
Bad:
<PRE><CODE>
int i = 10;
/* ... */
if (i) {
/* Do something. */
}
</CODE></PRE>
<H4>The Ternary Operator</H4>
The ?: 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.<P>
Good:
<PRE><CODE>
printf("%c is%s a number.\n", c, isdigit(c) ? "" " NOT");
l = (l1 < l2) ? l1 : l2;
if (gp.length + (go < 16384 ? 2 : 3) >= name->length) {
...
}
</CODE></PRE>
Bad:
<PRE><CODE>
return (success ? ISC_R_SUCESS : ISC_R_FAILURE);
</CODE></PRE>
<H4>Assignment in Parameters</H4>
Variables should not have their values assigned or changed when being
passed as parameters, except perhaps for the increment and decrement
operators.<P>
Bad:
<PRE><CODE>
malloc(size = 20);
</CODE></PRE>
Ok:
<PRE><CODE>
fputc(c++, stdout);
</CODE></PRE>
<H3>Namespace</H3>
<H4>Public Interfaces</H4>
All public interfaces to functions, macros, typedefs, and
variables provided by the library, should use names of the form
{library}_{module}_{what}, such as:
<PRE><CODE>
isc_buffer_t /* typedef */
dns_name_setbuffer(name, buffer) /* function */
ISC_LIST_HEAD(list) /* macro */
isc_commandline_argument /* variable */
</CODE></PRE>
however, structures which are typedef'd generally have the name of the
typedef sans the final _t:
<PRE><CODE>
struct dns_rbtnode {
/* ... members ... */
}
</CODE></PRE>
Generally speaking macros are defined with all capital letters, but
this is not universally consistent (eg, numerous isc_buffer_{foo}
macros).<P>
The {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).<P>
The one notable exception to this naming rule is the interfaces
provided by <isc/util.h>. 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.<P>
<H4>Shared Private Interfaces</H4>
When 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.
<H3>Initialization</H3>
When an object is allocated from the heap, all fields in the object must be
initialized.<P>
<H3>Dead Code Pruning</H3>
Source which becomes obsolete should be removed, not just disabled with
#if 0 ... #endif.<P>