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

diff --git a/doc/coding b/doc/coding
index 46fea16b4fcf9bdcd93cad5a50ffc92380fcca11..652c3deb787d7072fca97b22dac70da436c9048b 100644 (file)
--- a/doc/coding
+++ b/doc/coding
@@ -345,6 +345,14 @@ at this, please don't make it worse.
 
 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
@@ -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().
 
-  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 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.
 
-  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.
+  It's best to keep puts close to gets, both in runtime and in the
+  source code.
 
 * 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
   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
 ---