These guidelines don't attempt to be exhaustive. More complete
guidelines that are mostly compatible with Empire can be found at
-http://www.jetcafe.org/~jim/c-style.html
+<http://www.jetcafe.org/~jim/c-style.html>.
+
+See also doc/contributing.
Source tree organization
GNU indent.
We tried to restore things to the original style, mostly. There is
-one noteable change: basic indentation is now four spaces. Restoring
+one notable change: basic indentation is now four spaces. Restoring
the original eight spaces would have resulted in many more long lines,
which would have to be broken by indent. Since indent isn't good at
breaking lines tastefully, we reluctantly chose four instead.
`c-default-style' to "stroustrup" to switch to this style for all
buffers.
-Tab character use
+Avoid gratuitous space change
+
+Don't change whitespace gratuitiously, say just because your editor
+screws up tabs. Such changes make it much harder to figure out who
+changed what and when.
+
+TAB character use
-Whether you use tab characters or not doesn't really matter that much,
-but tab stops are every eight characters, period.
+Whether you use TAB characters or not doesn't really matter that much,
+but TAB stops are every eight characters, period.
Indentation, placement of braces, function name
Comments to the right of code should start in column 32 if possible
(counting from zero).
-Comment lines should be indented exactly like the code the belong to.
+Comment lines should be indented exactly like the code they belong to.
You are encouraged to format multi-line comments like this:
/*
- * Please use complete sentences, with proper grammer,
+ * Please use complete sentences, with proper grammar,
* capitalization and punctuation. Use two spaces between
* sentences.
*/
// C++/C99 comments
-because they are not portable C89.
-
-
Conditional compilation
Unless the conditional code is very short, please comment it like
Every file should have a file comment FIXME
Every function should have a function comment that describes what it
-does. FIXME elaborate. Writing such comments helps both future
-maintainers and yourself: if it's too hard to write a concise function
-comment, then your function is likely too complicated and could use a
-redesign.
+does, unless it's blatantly obvious. If the function is longer than a
+few lines, it's almost certainly not obvious.
+
+The function comment should serve as a contract: state the
+preconditions, side effects, return values. Make sure to cover error
+conditions.
+
+Writing such comments helps both future maintainers and yourself: when
+writing a concise function comment is too hard, then your function is
+likely too complicated and could use a redesign.
+
+Use @param to refer to parameters. Use func() to refer to functions
+or function-like macros. Use arr[] to refer to arrays. This is to
+make the references stand out and to avoid ambiguity.
+
+Example:
+
+ /*
+ * Read update schedule from file @fname.
+ * Put the first @n-1 updates after @t0 into @sched[] in ascending order,
+ * terminated with a zero.
+ * Use @anchor as initial anchor for anchor-relative times.
+ * Return 0 on success, -1 on failure.
+ */
+ int
+ read_schedule(char *fname, time_t sched[], int n, time_t t0, time_t anchor)
+
+When documenting a struct or union, use @member to refer to its
+members.
The existing code has very little useful comments, and it'll likely
take us years to fix it. Please don't make it harder than it already
typedef struct foo foo;
-Yes, this is incompatble with C++. Reducing the number of names for
+Yes, this is incompatible with C++. Reducing the number of names for
the same thing is more useful than compatibility to a programming
language we don't use.
obfuscates the purpose, and narrow variables could even overflow.
Just assign 1. A lot of cleanup remains to be done there.
+Null pointer constant
+
+Please use NULL for clarity, not just 0.
+
Type casts
Casting pointers to and from `void *' clutters the code and serves no
Factor out common code
-Do not gratuitiously duplicate code! Ken Stevens said it well, and
+Do not gratuitously duplicate code! Ken Stevens said it well, and
it's as relevant as ever:
Cut-and-Paste coding is by far the biggest problem that the current
Portability
-----------
-FIXME C89, POSIX
+Code for IEEE Std 1003.1-2001 (POSIX.1-2001) with the X/Open System
+Interfaces Extension. This standard includes ISO/IEC 9899:1999 (C99)
+by reference.
+
+Some systems may have flaws that require work-arounds. Use Autoconf
+to detect the flaws, and isolate the system-specific code, e.g. by
+providing a replacement function when the system's function is flawed.
+
+Keeping the Windows build working may require extending src/lib/w32/.
+
+Keep the client as portable as practical. In particular, stick to
+C89.
FIXME sizes, printf formats
FIXME conditional compilation is a last resort
-FIXME s_char
-
Robustness
----------
FIXME what to do on error
+Sanity checking
+
+In many places, the code checks for conditions that should not happen,
+and then tries to recover. This is sound defensive programming.
+Please use CANT_HAPPEN() and CANT_REACH() for this purpose, because
+they log the error condition, and optionally abort the program, or
+write out a core dump. This is called an "oops".
+
Buffer overruns
FIXME
* You can get a copy with getFOO() and write it back with putFOO().
- Yielding the processor invalidates the copy. In particular, if you
- yield the processor between get and put, and another thread changes
- the game data, then put will clobber that change, possibly resulting
- in a corrupted game.
+ Putting updates both the object in memory and the disk file.
+
+ Any change to the object invalidates the copy. Putting such an
+ invalid copy will clobber the change(s) that invalidated it,
+ possibly resulting in corrupted game state. The code oopses on such
+ puts, but it can't repair the damage.
- Therefore, you have to re-get after a yield, and repeat any checks
- you already made. If you can afford to bail out when something
- changed at all, use check_FOO_ok().
+ There are two common ways to invalidate a copy: calling a function
+ that updates the object you copied (unless it does that through your
+ copy), and yielding the processor, which lets other threads update
+ the object you copied.
- Putting updates both the object in memory and the disk file.
+ Therefore, you have to re-get after a possible invalidation, and
+ deal with changes. In particular, if you checked whether the object
+ is suitable for a task, you need to check again after re-getting it.
+ If you can afford to bail out when something changed, use
+ check_FOO_ok().
- If you write a function that takes a pointer to an object, think
- twice before you put it, and if you do, mention it in the function
- comment.
+ Function comments should state what objects the function can update.
+ Unfortunately, they generally don't.
+
+ It's best to keep puts close to gets, both at runtime and in the
+ source code.
* Bmaps have special access functions.
you yield. If you only updated the working bmap, then you can call
writebmap() instead.
+Reading player input
-CVS
----
+Reading player input can fail, and you must check for that.
+Neglecting it can break the interrupt feature for players (normally
+Ctrl-C), and produce extra prompts that could confuse clients. Even
+worse is neglecting it in a loop that terminates only when input is
+read successfully.
-Commit related changes together, unrelated changes separately.
+When reading input fails, you should normally abort the command with
+status RET_SYN. There are exceptions, e.g. when aborting a pinpoint
+bombing run over the target.
-See chapter Change Logs in the GNU coding standards for guidelines on
-log messages. Try http://www.gnu.org/prep/standards_40.html#SEC40 or
-search the web for `gnu coding standards change logs'.
+Some functions to read player input return a special error value you
+can check, e.g. recvclient(), prmptrd() and uprmptrd() return a
+negative value, getstring() and getstarg() return NULL.
-Don't change whitespace gratuitiously, say just because your editor
-screws up tabs. Such changes make it much harder to figure out who
-changed what and when.
+Others return the same error value for failed read and for successful
+read of input that is invalid. Then you need to check
+player->aborted; if it is set after a read, the read failed.
\f
Historical guidelines, superseded by the above
Sasha Mikheev on indentation:
-The empire indentation style can be achived by using
+The empire indentation style can be achived by using
indent -orig -i8 foo.c
or in old c-mode emacs (versions before 19.29):
- There should always be a space on either side of a {
- There should always be a new line after a ;
- The closing function bracket should be on a line by itself
-
-
projects / empserver / blobdiff