[Freeciv-Dev] PATCH: remove check_coords (PR#1013)

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

This patch removes the check_coords function, in preparation for it to
be replaced by a better check_map_pos macro/function.

None of the current places that use check_coords is using it correctly,
anyway.

Note that in one place I have done

-  /* It is ok placing check_coords() inside an assert (ie may be
optimized
-     away by compiler). Though it may adjust dest_x, dest_y it
shouldn't
-     be needed. */
-  assert(check_coords(&dest_x, &dest_y));
+  assert(is_normal_map_pos(dest_x, dest_y));

Although the comment claims that the earlier code is correct, the
comment is wrong and the earlier code is a bug.  check_coords returns
TRUE any time the passed coordinates are real, regardless of whether
they had to be normalized.  Thus the original code was nearly the
equivalent of assert(normalize_map_pos(&dest_x, &dest_y)).  It's
probably true that this doesn't matter since the coordinates are always
normal.

Raimar: we had discussed writing the check_map_pos macro before removing
check_coords, but since the check_coords removal is more straightforward
I made this patch before.  If you still want to do the other change
first, this one can wait.  Note that only in two situations (the one
above and possibly one other) should check_map_pos be substituted in for
check_coords, so there's not as much conflict between these two changes
as you might expect.

jason

? rc
? check_coords
? old
? topology
Index: common/map.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
retrieving revision 1.96
diff -u -r1.96 map.c
--- common/map.c        2001/10/11 12:37:04     1.96
+++ common/map.c        2001/10/14 05:23:50
@@ -1289,25 +1289,6 @@
   return (x1 == x2 && y1 == y2);
 }
 
-/**************************************************************************
-...
-**************************************************************************/
-int check_coords(int *x, int *y)
-{
-  int save_x = *x, save_y = *y;
-
-  if (!normalize_map_pos(x, y)) {
-    freelog(LOG_ERROR, "%d, %d is not a real tile!", *x, *y);
-    nearest_real_pos(x, y);
-    return 0;
-  }
-
-  if (*x != save_x || *y != save_y)
-    freelog(LOG_ERROR, "Had to adjust coords %d, %d", *x, *y);
-
-  return 1;
-}
-
 int is_real_tile(int x, int y)
 {
   int x1 = x, y1 = y;
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.96
diff -u -r1.96 map.h
--- common/map.h        2001/10/09 16:26:46     1.96
+++ common/map.h        2001/10/14 05:23:50
@@ -229,7 +229,6 @@
 void map_clear_special(int x, int y, enum tile_special_type spe);
 void tile_init(struct tile *ptile);
 enum known_type tile_is_known(int x, int y);
-int check_coords(int *x, int *y);
 int is_real_tile(int x, int y);
 int is_normal_map_pos(int x, int y);
 int normalize_map_pos(int *x, int *y);
Index: server/unithand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unithand.c,v
retrieving revision 1.205
diff -u -r1.205 unithand.c
--- server/unithand.c   2001/10/04 20:09:29     1.205
+++ server/unithand.c   2001/10/14 05:23:51
@@ -67,8 +67,8 @@
 {
   struct unit *punit;
 
-  if (!check_coords(&req->x, &req->y))
-    return;
+  if (!normalize_map_pos(&req->x, &req->y))
+    return; /* discard invalid packet */
 
   if((punit=player_find_unit_by_id(pplayer, req->unit_id))) {
     punit->goto_dest_x=req->x;
@@ -97,8 +97,8 @@
   struct unit *punit;
   struct city *pcity;
 
-  if (!check_coords(&req->x, &req->y))
-    return;
+  if (!normalize_map_pos(&req->x, &req->y))
+    return; /* discard invalid packet */
 
   punit = player_find_unit_by_id(pplayer, req->unit_id);
   pcity = map_get_city(req->x, req->y);
@@ -117,8 +117,8 @@
 {
   struct unit *punit;
 
-  if (!check_coords(&req->dest_x, &req->dest_y))
-    return;
+  if (!normalize_map_pos(&req->dest_x, &req->dest_y))
+    return; /* discard invalid packet */
 
   if((punit=player_find_unit_by_id(pplayer, req->unit_id))) {
     if (can_unit_do_connect (punit, req->activity_type)) {
@@ -572,8 +572,8 @@
 
   if(punit) {
     if (!same_pos(punit->x, punit->y, pinfo->x, pinfo->y)) {
-      if (!check_coords(&pinfo->x, &pinfo->y))
-       return;
+      if (!normalize_map_pos(&pinfo->x, &pinfo->y))
+       return; /* discard invalid packet */
 
       if (is_tiles_adjacent(punit->x, punit->y, pinfo->x, pinfo->y)) {
        punit->ai.control = 0;
@@ -607,8 +607,8 @@
 {
   struct unit *punit;
 
-  if (!check_coords(&pmove->x, &pmove->y))
-    return;
+  if (!normalize_map_pos(&pmove->x, &pmove->y))
+    return; /* discard invalid packet */
 
   punit=player_find_unit_by_id(pplayer, pmove->unid);
   if (punit && is_tiles_adjacent(punit->x, punit->y, pmove->x, pmove->y)) {
@@ -835,7 +835,7 @@
     return 0;
   }
 
-  if (!check_coords(&dest_x, &dest_y))
+  if (!normalize_map_pos(&dest_x, &dest_y))
     return 0;
 
   /* this occurs often during lag, and to the AI due to some quirks -- Syela */
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.140
diff -u -r1.140 unittools.c
--- server/unittools.c  2001/10/06 11:54:31     1.140
+++ server/unittools.c  2001/10/14 05:23:53
@@ -2895,10 +2895,7 @@
   struct tile *psrctile = map_get_tile(src_x, src_y);
   struct tile *pdesttile = map_get_tile(dest_x, dest_y);
 
-  /* It is ok placing check_coords() inside an assert (ie may be optimized
-     away by compiler). Though it may adjust dest_x, dest_y it shouldn't
-     be needed. */
-  assert(check_coords(&dest_x, &dest_y));
+  assert(is_normal_map_pos(dest_x, dest_y));
 
   conn_list_do_buffer(&pplayer->connections);