Update coding guidelines: oops, get/put, reading player input

This commit is contained in:
Markus Armbruster 2008-07-27 12:02:34 -04:00
parent 8e9cbf4ab1
commit a80489de39

View file

@ -345,6 +345,14 @@ at this, please don't make it worse.
FIXME what to do on error 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 Buffer overruns
FIXME FIXME
@ -394,20 +402,29 @@ There are several ways to access an object in set FOO:
* You can get a copy with getFOO() and write it back with putFOO(). * 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.
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().
Putting updates both the object in memory and the disk file. Putting updates both the object in memory and the disk file.
If you write a function that takes a pointer to an object, think Any change to the object invalidates the copy. Putting such an
twice before you put it, and if you do, mention it in the function invalid copy will clobber the change(s) that invalidated it,
comment. possibly resulting in corrupted game state. The code oopses on such
puts, but it can't repair the damage.
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.
Therefore, you have to reget 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 regetting it. If
you can afford to bail out when something changed, use
check_FOO_ok().
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 in runtime and in the
source code.
* Bmaps have special access functions. * Bmaps have special access functions.
@ -421,6 +438,26 @@ There are several ways to access an object in set FOO:
you yield. If you only updated the working bmap, then you can call you yield. If you only updated the working bmap, then you can call
writebmap() instead. writebmap() instead.
Reading player input
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.
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.
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.
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.
git git
--- ---