diff --git a/doc/dev/DBC b/doc/dev/DBC new file mode 100644 index 0000000000..0e2d11edfb --- /dev/null +++ b/doc/dev/DBC @@ -0,0 +1,31 @@ + +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. + diff --git a/doc/dev/coding.html b/doc/dev/coding.html new file mode 100644 index 0000000000..3dada5b820 --- /dev/null +++ b/doc/dev/coding.html @@ -0,0 +1,362 @@ +

C Language

+ +An ANSI standard C compiler and library are assumed. Feel free to use any +ANSI C feature.

+ +

Warnings

+Given a reasonable set of things to warn about (e.g. -W -Wall for gcc), the +goal is to compile with no warnings. + +

C Source Code

+ +

Copyright

+ +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.

+ +

Line Formatting

+

Indentation

+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 "_": +

+	printf("this is going to be %s very long %s statement\n",
+	_______"a", "printf");
+
+ +

Line Length

Lines should not be longer than 80 characters, +even if it requires violating the indentation rules to do so. + +

Comments

+Comments should be used anytime they improve the readability of the code.

+ +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.

+Good:

+


+	/*
+	 * Private variables.
+	 */
+
+	static int		a		/* Description of 'a'. */
+	static int		b		/* Description of 'b'. */
+	static char *		c		/* Description of 'c'. */
+
+ +The following lint and lint-like comments should be used where appropriate: +

+	/* ARGSUSED */
+	/* FALLTHROUGH */
+	/* NOTREACHED */
+	/* VARARGS */
+
+ +

.h files

+.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.

+.h files that define modules should have a structure like the following:

+


+/*
+ * 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
+ *****/
+
+/*
+ * 
+ *
+ * 
+ *
+ * 
+ *
+ * MP:
+ *	
+ *
+ * Reliability:
+ *	
+ *
+ * Resources:
+ *	
+ *
+ * Security:
+ *	
+ *
+ * Standards:
+ *	
+ */
+
+/***
+ *** Imports
+ ***/
+
+/* <#includes here> */
+
+/***
+ *** Types
+ ***/
+
+/*  */
+
+/***
+ *** Functions
+ ***/
+
+#endif /* ISC_WHATEVER_H */
+
+
+ +

C Source

+

Including Interfaces (.h files)

+The first file to be included must be config.h. +Try to include only necessary files, not everything under the +sun.

+Operating-system-specific files should not be included by most modules.

+Include UNIX "sys" .h files before ordinary C includes.

+ +

Statements

+There should be at most one statement per line.

+Bad:

+


+	if (i > 0) {
+		printf("yes\n"); i = 0; j = 0;
+	}
+
+

Functions

+The use of ANSI C function prototypes is required.

+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.

+Good:

+static inline void +f(int i) { + /* whatever */ +} + +int +g(int i, /* other args here */ + int last_argument) +{ + return (i * i); +} + + +

Curly Braces

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.

+Good:

+


+static void
+f(int i) {
+	if (i > 0) {
+		printf("yes\n");
+		i = 0;
+	} else
+		printf("no\n");
+}
+
+Bad:

+


+void f(int i)
+  {
+    if(i<0){i=0;printf("was negative\n");}
+    if (i > 0)
+      {
+        printf("yes\n");
+        i = 0;
+      }}
+
+ +

Spaces

+ + + +

Return Values

+If a function returns a value, it should be cast to (void) if you don't +care what the value is. (Exception for printf()?)

+ +All error conditions must be handled.

+ +Mixing of error status and valid results within a single type should be +avoided.

+Good: +


+	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;
+	}
+
+Not so good: +

+	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;
+	}
+
+ +

Integral Types

+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.

+ +

Casting

+Casting should be avoided when possible.

+ +

Clear Success or Failure

+A function should report success or failure, and do so accurately. It +should never fail silently. Use of Design by Contract can help here.

+ +

Testing Bits

+Bit testing should be as follows:

+Good: +


+	/* 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)) {
+
+	}
+
+Bad: +

+	/* Test if flag set. */
+	if (flags & FOO) {
+
+	}
+	/* Test if flag clear. */
+	if (! (flags & BAR)) {
+
+	}
+
+ +

Pointers

+
Null Pointer
+The null pointer value should be referred to with "NULL", not with "0". +Testing to see whether a pointer is NULL should be explicit.

+Good: +


+	char *c = NULL;
+
+	/* ... */
+
+	if (c == NULL) {
+		/* Do something. */
+	}
+
+ +
Invalidating Pointers
+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.

+Good: +


+	char *text;
+
+	/* text is initalized here. */
+
+	free(text);
+	text = NULL;
+
+ +

Testing for Zero or Non-zero

+Explicit testing against zero is required for numeric, non-boolean variables. +

+Good: +


+	int i = 10;
+
+	/* ... */
+
+	if (i != 0) {
+		/* Do something. */
+	}
+
+Bad: +

+	int i = 10;
+
+	/* ... */
+
+	if (i) {
+		/* Do something. */
+	}
+
+ +

Initialization

+When an object is allocated from the heap, all fields in the object must be +initialized.

+ +

Dead Code Pruning

+Source which becomes obsolete should be removed, not just disabled with +#if 0 ... #endif.

diff --git a/doc/dev/magic_numbers b/doc/dev/magic_numbers new file mode 100644 index 0000000000..dc16f1b9b6 --- /dev/null +++ b/doc/dev/magic_numbers @@ -0,0 +1,60 @@ + +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)); + } diff --git a/doc/dev/results b/doc/dev/results new file mode 100644 index 0000000000..f4fb57a746 --- /dev/null +++ b/doc/dev/results @@ -0,0 +1,57 @@ + +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. diff --git a/doc/dev/unexpected b/doc/dev/unexpected new file mode 100644 index 0000000000..cd54f09031 --- /dev/null +++ b/doc/dev/unexpected @@ -0,0 +1,44 @@ + +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 + +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.