Commit 767d29c4 authored by Bob Halley's avatar Bob Halley
Browse files

add

parent ffea097e
Design By Contract
BIND 9 uses the "Design by Contract" idea for most function calls.
A quick summary of the idea is that a function and its caller make a
contract. If the caller meets certain preconditions, then the
function promises to either fulfill its contract (i.e. guarantee a set
of postconditions), or to clearly fail.
"Clearly fail" means that if the function cannot succeed, then it will
not silently fail and return a value which the caller might interpret
as success.
If a caller doesn't meet the preconditions, then "further execution is
undefined". The function can crash, compute a garbage result, fail silently,
etc. Allowing the function to define preconditions greatly simplifies many
APIs, because the API need not have a way of saying "hey caller, the values
you passed in are garbage".
Typically, preconditions are specified in the functions .h file, and encoded
in its body with REQUIRE statements. The REQUIRE statements cause the program
to dump core if they are not true, and can be used to identify callers that
are not meeting their preconditions.
Postconditions can be encoded with ENSURE statements. Within the body of
a function, INSIST is used to assert that a particular expression must be
true. Assertions must not have side effects that the function relies upon,
because assertion checking can be turned off.
<H2>C Language</H2>
An ANSI standard C compiler and library are assumed. Feel free to use any
ANSI C feature.<P>
<H2>Warnings</H2>
Given a reasonable set of things to warn about (e.g. -W -Wall for gcc), the
goal is to compile with no warnings.
<H2>C Source Code</H2>
<H3>Copyright</H3>
All 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.<P>
<H3>Line Formatting</H3>
<H4>Indentation</H4>
Use tabs. Spaces are only allowed when needed to line up a continued
expression. In the following example, spaces used for indentation are
indicated with "_":
<PRE><CODE>
printf("this is going to be %s very long %s statement\n",
_______"a", "printf");
</CODE></PRE>
<H4>Line Length</H4> Lines should not be longer than 80 characters,
even if it requires violating the indentation rules to do so.
<H3>Comments</H3>
Comments should be used anytime they improve the readability of the code.<P>
Comments may be single-line or multiline. A single-line comment should be
at the end of the line of 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.<P>
Good:<P>
<PRE><CODE>
/*
* Private variables.
*/
static int a /* Description of 'a'. */
static int b /* Description of 'b'. */
static char * c /* Description of 'c'. */
</CODE></PRE>
The following lint and lint-like comments should be used where appropriate:
<PRE><CODE>
/* ARGSUSED */
/* FALLTHROUGH */
/* NOTREACHED */
/* VARARGS */
</CODE></PRE>
<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:<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> */
/***
*** Types
***/
/* <Type definitions here> */
/***
*** Functions
***/
#endif /* ISC_WHATEVER_H */
</CODE></PRE>
<H3>C Source</H3>
<H4>Including Interfaces (.h files)</H4>
The first file to be included must be config.h.
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.<P>
Bad:<P>
<PRE><CODE>
if (i > 0) {
printf("yes\n"); i = 0; j = 0;
}
</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>
static inline void
f(int i) {
/* whatever */
}
int
g(int i, /* other args here */
int last_argument)
{
return (i * i);
}
</CODE></PRE>
<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>
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 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.
</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. (Exception 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.<P>
<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>
<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>
Magic Numbers
A number of data structures in the ISC and DNS libraries have an unsigned int
magic number as the first field. The purpose of the magic number is
principally to validate that a pointer a subroutine has gotten really points
to the type it claims to be. This helps detect problems caused by resources
being freed prematurely, that have been corrupted, or that have not been
properly initalized. It can also be handy in debugging.
Magic numbers should always be the first field. They never require locking
to access. As to the actual value to be used, something mnemonic is good:
#define TASK_MAGIC 0x5441534BU /* TASK. */
#define VALID_TASK(t) ((t) != NULL && \
(t)->magic == TASK_MAGIC)
#define TASK_MANAGER_MAGIC 0x54534B4DU /* TSKM. */
#define VALID_MANAGER(m) ((m) != NULL && \
(m)->magic ==
TASK_MANAGER_MAGIC)
Unless the memory cost is critical, most objects should have a magic number.
The magic number should be the last field set in a creation routine, so that
an object will never be stamped with a magic number unless it is valid.
The magic number should be set to zero immediately before the object is
freed.
Magic values are generally private to the implementation of the type. I.e.
they are defined in the .c file, not the .h file.
Validation of magic numbers is done by routines that manipulate the type,
not by users of the type. Indeed, user validation is usually not possible
because the magic number is not public.
Magic number checking may become a build option in a future release. E.g.
struct foo {
ISC_MAGIC_DECLARATION
/* ... */
}
foo_create() {
/* ... */
ISC_MAGIC_SET(value);
}
foo_destroy() {
/* ... */
ISC_MAGIC_CLEAR(value);
}
#define FOO_MAGIC 0x00010203U
#define VALID_FOO(f) ISC_MAGIC_VALIDATE(f, FOO_MAGIC)
foo_dosomething(foo *f) {
REQUIRE(VALID_FOO(f));
}
Result Codes
The use of global variables or a GetLastError() function to return results
doesn't work well in a multithreaded application. The global variable has
obvious problems, as does a global GetLastError(). A per-object GetLastError()
seems more promising, e.g.
sometype_t s;
sometype_dosomething(s, buffer);
if (sometype_error(s)) {
/* ... */
}
If 's' is shared however this approach doesn't work unless the locking is
done by the caller, e.g.
sometype_lock();
sometype_dosomething(s, buffer);
if (sometype_error(s)) {
/* ... */
}
sometype_unlock();
Those ISC and DNS libraries which have locks almost universally put the
locking inside of the called routines, since it's more convenient for
the calling programmer, makes for a cleaner API, and puts the burden
of locking on the library programmer, who should know best what the
locking needs of the routine are.
Because of this locking style the ISC and DNS libraries typically provide
result information as the return value of the function. E.g.
isc_result_t result;
result = isc_task_send(task, &event);
Note that an explicit result type is used, instead of mixing the error result
type with the normal result type. E.g. the C library routine getc() can
return a character or EOF, but the BIND 9 style keeps the types of the
function's return values separate.
char c;
result = isc_io_getc(stream, &c);
if (result == ISC_R_SUCCESS) {
/* Do something with 'c'. */
} else if (result == ISC_R_EOF) {
/* EOF. */
} else {
/* Some other error. */
}
Functions which cannot fail (assuming the caller has provided valid
arguments) need not return a result type. For example, dns_name_issubdomain()
returns an isc_boolean_t, because it cannot fail.
Unexpected Errors
For portability, the ISC and DNS libraries define their own result codes
instead of using the operating system's. E.g. the ISC library uses
ISC_R_NOMEMORY instead of the UNIX-specific ENOMEM.
The ISC and DNS libraries have a common way of looking at errors and
other non-success results. An "expected" result is something that can
happen in the ordinary course of using a function, that is not very
improbable, and that the caller might care to know. For example, a
function which opens a file must have a way to say "file not found"
and "permission denied".
Other kinds of errors are "unexpected". For example, an I/O error
might occur. When an unexpected error occurs, we want to be able to
log the information, but we don't want to translate every
operating-system-specific error code into and ISC_R_ or DNS_R_ code
because the are too many of them, and they aren't meaningful to
clients anyway (they're unexpected errors). If we were using a
language where we could throw an exception, we'd do that. Since we're
not, we call UNEXPECTED_ERROR(). E.g.
#include <isc/error.h>
void foo() {
if (some_unix_thang() < 0) {
UNEXPECTED_ERROR(__FILE__, __LINE__,
"some_unix_thang() failed: %s",
strerror(errno));
return (ISC_R_UNEXPECTED);
}
}
The UNEXPECTED error routine may be specified by the calling application. It
will log the error somehow (e.g. via. syslog, or printing to stderr).
This method is a compromise. It makes useful error information available,
but avoids the complexity of a more sophisticated multi-library "error table"
scheme.
In the (rare) situation where a library routine encounters a fatal error and
has no way of reporting the error to the application, the library may call
FATAL_ERROR(). This will log the problem and then terminate the application.
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment