[Freeciv-Dev] resubmission: bug in vnotify_conn_ex (PR#1185)

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

This is a resubmission of the patch to fix a future bug in vnotify_conn_ex.

The bug is two-fold:

- is_real_tile and friends should not be called unless the game has 
started.  Thus, the check for server_state >= RUN_GAME_STATE has been 
moved up.
- (-1,-1) is a special case meaning that the event has no associated 
coordinates.  In the future it may be possible for (-1,-1) to be a real 
position; however, it is assumed that it will not be a normal position 
(otherwise this code will completely break).  Thus I have added two 
assertions: one to assert that (-1,-1) is a non-normal position, and 
another to assert that any position other than (-1,-1) that is passed in 
is normal.  The latter is needed for debugging: if it is possible for a 
unnormalized real position to be passed in, then (-1,-1) could be passed 
in by mistake; it would be incorrectly interpreted as the special-case 
when in fact a real position was intended.  (The assertion that 
!is_normal_map_pos(-1,-1) may be considered spurious but helps to ensure 
that this code is not inadvertantly broken in the future by changes 
elsewhere.)
Ross objected to this change when it was submitted before, but he did 
not understand the change I was making.  Ross, if you still object will 
you (1) justify the objection (make sure you understand the change 
first) and (2) suggest an alternate change.
This code needs to be changed before alternate topologies can be 
realized.  There are several other possibilities on how to change it (we 
could call normalize_map_pos in the process, though this is not strictly 
necessary); however I feel strongly that at least one assert is needed 
(to make sure any coordinates other than (-1,-1) that are passed in are 
normal), and the server_state check does need to be moved up.
jason

? foo
? get_data_dirs.diff
? tileset_list.diff
Index: server/plrhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/plrhand.c,v
retrieving revision 1.238
diff -u -r1.238 plrhand.c
--- server/plrhand.c    2002/07/11 17:04:04     1.238
+++ server/plrhand.c    2002/07/20 18:44:54
@@ -944,7 +944,8 @@
   and specified (x,y) coords associated with the event.  Coords will only
   apply if game has started and the conn's player knows that tile (or
   pconn->player==NULL && pconn->observer).  If coords are not required,
-  caller should specify (x,y) = (-1,-1).  For generic event use E_NOEVENT.
+  caller should specify (x,y) = (-1,-1); otherwise make sure that the
+  coordinates have been normalized.  For generic event use E_NOEVENT.
   (But current clients do not use (x,y) data for E_NOEVENT events.)
 **************************************************************************/
 static void vnotify_conn_ex(struct conn_list *dest, int x, int y, int event,
@@ -956,12 +957,15 @@
   genmsg.event = event;
 
   conn_list_iterate(*dest, pconn) {
-    if (is_real_tile(x, y) && server_state >= RUN_GAME_STATE
+    if (server_state >= RUN_GAME_STATE
+       && (x != -1 || y != -1) /* special case, see above */
        && ((!pconn->player && pconn->observer)
            || (pconn->player && map_get_known(x, y, pconn->player)))) {
+      CHECK_MAP_POS(x, y);
       genmsg.x = x;
       genmsg.y = y;
     } else {
+      assert(server_state < RUN_GAME_STATE || !is_normal_map_pos(-1, -1));
       genmsg.x = -1;
       genmsg.y = -1;
     }