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

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

This buglet is very similar to PR#1184.

The function vnotify_conn_ex() notifies a player of an event.  A 
coordinate set can optionally be passed in to give the location of the 
event, or (-1,-1) can be passed to identify a "no-location" event.  This 
is a small hack, but nothing much to worry about.
There are two problems with the internals of vnotify_conn_ex(), though. 
 Both deal with this line:
    if (is_real_tile(x, y) && server_state >= RUN_GAME_STATE

1.  Calling is_real_tile before the game starts is generally no good. 
Under general-topologies, it will result in a failed assertion as the 
topology will not yet have been initialized.  Thus it should be clear 
that the order here should be reversed.
2.  Again, is_real_tile may return true for (-1,-1) under an alternate 
topology.  The easy solution here is just to change it to 
is_normal_map_pos.  There are still some minor pitfalls: (1) it 
obviously won't work if is_normal_map_pos(-1,-1), and (2) we need to be 
careful about non-normal positions other than (-1,-1) being passed in; 
these would probably indicate a bug.  Both of these can be detected with 
a well-placed assertion.
The attached patch fixes both of these issues.

Note, the handling here is very similar to that in Ross's corecleanups. 
 I believe this way to be slightly more correct, as it will catch the 
two pitfall potential bugs mentioned above.
jason

Index: server/plrhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/plrhand.c,v
retrieving revision 1.203
diff -u -r1.203 plrhand.c
--- server/plrhand.c    2001/12/21 11:17:44     1.203
+++ server/plrhand.c    2002/01/02 08:41:17
@@ -893,7 +893,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 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,
@@ -905,12 +906,16 @@
   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 && is_normal_map_pos(x, y)
        && ((pconn->player==NULL && pconn->observer)
            || (pconn->player!=NULL && map_get_known(x, y, pconn->player)))) {
+      /* If is_normal_map_pos(-1,-1), this assertion will catch the bug. */
+      assert(x != -1 || y != -1);
       genmsg.x = x;
       genmsg.y = y;
     } else {
+      /* Any invalid coordinates other than (-1, -1) are a bug. */
+      assert((x == -1 && y == -1) || is_normal_map_pos(x, y));
       genmsg.x = -1;
       genmsg.y = -1;
     }