Complete.Org: Mailing Lists: Archives: freeciv-dev: November 2001:
[Freeciv-Dev] Re: [PATCH] check_map_pos change (PR#1031)
Home

[Freeciv-Dev] Re: [PATCH] check_map_pos change (PR#1031)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: freeciv-dev@xxxxxxxxxxx
Cc: bugs@xxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: [PATCH] check_map_pos change (PR#1031)
From: jdorje@xxxxxxxxxxxxxxxxxxxxx
Date: Thu, 1 Nov 2001 04:57:19 -0800 (PST)

Gaute B Strokkenes wrote:
> 
> On Tue, 30 Oct 2001, hawk@xxxxxxxxxxxxxxxxxxxxxxx wrote:
> > On Tue, Oct 30, 2001 at 02:45:01AM +0000, Gaute B Strokkenes wrote:
> >> On Mon, 29 Oct 2001, vze2zq63@xxxxxxxxxxx wrote:
> >>
> >> I propose the following:

With this one I have taken Gaute's most recent patch and made the
following changes:

1.  Renamed CHECK_POS as CHECK_MAP_POS.

2.  Changed the #definition of CHECK_POS as Raimar suggested.

3.  Moved the "#include <assert.h>" into map.h.

4.  Changed existing assert(is_normal_map_pos(...)) to use CHECK_MAP_POS
(or removed them, if appropriate).

5.  Added CHECK_MAP_POS checks into all the map iterator macros (not
city or unit ones).

6.  Added CHECK_MAP_POS checks to is_tiles_adjacent.

My rule is this: all coordinates must be checked.  A function may only
forego the check if it is passing the coordinates off to another
function that it knows will do the check.  This leads to unfortunate
dependencies, but the alternative is either checks everywhere or not
enough checks.

jason
? rc
? old
? topology
? corecleanup_08.ReadMe
? civserver-regular
Index: client/tilespec.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/tilespec.c,v
retrieving revision 1.56
diff -u -r1.56 tilespec.c
--- client/tilespec.c   2001/10/26 07:24:55     1.56
+++ client/tilespec.c   2001/11/01 12:56:34
@@ -1114,7 +1114,6 @@
 
   *solid_bg = 0;
 
-  assert(is_normal_map_pos(x, y));
   if (!tile_is_known(x, y))
     return -1;
 
@@ -1341,7 +1340,6 @@
   *solid_bg = 0;
   *pplayer = NULL;
 
-  assert(is_normal_map_pos(abs_x0, abs_y0));
   ptile=map_get_tile(abs_x0, abs_y0);
 
   if (tile_is_known(abs_x0,abs_y0) == TILE_UNKNOWN) {
Index: common/map.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
retrieving revision 1.98
diff -u -r1.98 map.c
--- common/map.c        2001/10/30 10:59:19     1.98
+++ common/map.c        2001/11/01 12:56:35
@@ -1068,9 +1068,6 @@
   int maxcost = 72; /* should be big enough without being TOO big */
   struct tile *tile0, *tile1;
 
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
-
   tile0 = map_get_tile(x, y);
   debug_log_move_costs("Resetting move costs for", x, y, tile0);
 
@@ -1128,6 +1125,8 @@
 ***************************************************************/
 int is_tiles_adjacent(int x0, int y0, int x1, int y1)
 {
+  CHECK_MAP_POS(x1, y1);
+  CHECK_MAP_POS(x0, y0); /* duplicated in adjc_iterate */
   adjc_iterate(x0, y0, x, y) {
     if (x == x1 && y == y1)
       return 1;
@@ -1156,10 +1155,6 @@
 ***************************************************************/
 struct tile *map_get_tile(int x, int y)
 {
-  int is_real = normalize_map_pos(&x, &y);
-
-  assert(is_real);
-
   return MAP_TILE(x, y);
 }
 
@@ -1168,10 +1163,7 @@
 ***************************************************************/
 signed short map_get_continent(int x, int y)
 {
-  if (!normalize_map_pos(&x, &y))
-    return -1;
-  else
-    return MAP_TILE(x, y)->continent;
+  return MAP_TILE(x, y)->continent;
 }
 
 /***************************************************************
@@ -1179,8 +1171,6 @@
 ***************************************************************/
 void map_set_continent(int x, int y, int val)
 {
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
   MAP_TILE(x, y)->continent = val;
 }
 
@@ -1190,10 +1180,7 @@
 ***************************************************************/
 enum tile_terrain_type map_get_terrain(int x, int y)
 {
-  if (!normalize_map_pos(&x, &y))
-    return T_UNKNOWN;
-  else
-    return MAP_TILE(x, y)->terrain;
+  return MAP_TILE(x, y)->terrain;
 }
 
 /***************************************************************
@@ -1201,10 +1188,7 @@
 ***************************************************************/
 enum tile_special_type map_get_special(int x, int y)
 {
-  if (!normalize_map_pos(&x, &y))
-    return S_NO_SPECIAL;
-  else
-    return MAP_TILE(x, y)->special;
+  return MAP_TILE(x, y)->special;
 }
 
 /***************************************************************
@@ -1212,8 +1196,6 @@
 ***************************************************************/
 void map_set_terrain(int x, int y, enum tile_terrain_type ter)
 {
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
   MAP_TILE(x, y)->terrain = ter;
 }
 
@@ -1222,9 +1204,6 @@
 ***************************************************************/
 void map_set_special(int x, int y, enum tile_special_type spe)
 {
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
-
   MAP_TILE(x, y)->special |= spe;
 
   if (spe & (S_ROAD | S_RAILROAD))
@@ -1236,8 +1215,6 @@
 ***************************************************************/
 void map_clear_special(int x, int y, enum tile_special_type spe)
 {
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
   MAP_TILE(x, y)->special &= ~spe;
 
   if (spe & (S_ROAD | S_RAILROAD))
@@ -1249,8 +1226,6 @@
 ***************************************************************/
 struct city *map_get_city(int x, int y)
 {
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
   return MAP_TILE(x, y)->city;
 }
 
@@ -1260,8 +1235,6 @@
 ***************************************************************/
 void map_set_city(int x, int y, struct city *pcity)
 {
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
   MAP_TILE(x, y)->city = pcity;
 }
 
@@ -1271,10 +1244,7 @@
 ***************************************************************/
 enum known_type tile_is_known(int x, int y)
 {
-  if (!normalize_map_pos(&x, &y))
-    return TILE_UNKNOWN;
-  else
-    return (enum known_type) (MAP_TILE(x, y)->known);
+  return (enum known_type) (MAP_TILE(x, y)->known);
 }
 
 /***************************************************************
@@ -1283,17 +1253,14 @@
 ***************************************************************/
 int same_pos(int x1, int y1, int x2, int y2)
 {
-  assert(is_real_tile(x1, y1) && is_real_tile(x2, y2));
-  normalize_map_pos(&x1, &y1);
-  normalize_map_pos(&x2, &y2);
+  CHECK_MAP_POS(x1, y1);
+  CHECK_MAP_POS(x2, y2);
   return (x1 == x2 && y1 == y2);
 }
 
 int is_real_tile(int x, int y)
 {
-  int x1 = x, y1 = y;
-
-  return normalize_map_pos(&x1, &y1);
+  return normalize_map_pos(&x, &y);
 }
 
 /**************************************************************************
@@ -1436,9 +1403,6 @@
 **************************************************************************/
 int is_move_cardinal(int start_x, int start_y, int end_x, int end_y)
 {
-  assert(is_real_tile(start_x, start_y) && is_real_tile(end_x, end_y));
-  normalize_map_pos(&start_x, &start_y);
-  normalize_map_pos(&end_x, &end_y);
   assert(is_tiles_adjacent(start_x, start_y, end_x, end_y));
 
   /* FIXME: this check will not work with an orthogonal map */
Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.101
diff -u -r1.101 map.h
--- common/map.h        2001/10/30 10:59:20     1.101
+++ common/map.h        2001/11/01 12:56:37
@@ -13,6 +13,7 @@
 #ifndef FC__MAP_H
 #define FC__MAP_H
 
+#include "assert.h"
 #include "player.h"
 #include "terrain.h"
 #include "unit.h"
@@ -197,6 +198,9 @@
 void initialize_move_costs(void);
 void reset_move_costs(int x, int y);
 
+#define CHECK_MAP_POS(x,y) \
+  (assert(is_normal_map_pos((x),(y))), TRUE)
+
 #define map_adjust_x(X)            \
   ((X) < 0                         \
    ? ((X) % map.xsize != 0 ? (X) % map.xsize + map.xsize : 0) \
@@ -208,7 +212,7 @@
   (((Y)<0) ? 0 : (((Y)>=map.ysize) ? map.ysize-1 : (Y)))
 
 #define map_inx(x,y) \
-  ((x)+(y)*map.xsize)
+  (CHECK_MAP_POS((x),(y)), (x)+(y)*map.xsize)
 
 #define DIRSTEP(dest_x, dest_y, dir)   \
 (    (dest_x) = DIR_DX[(dir)],         \
@@ -367,6 +371,7 @@
   int MACRO_xcycle = 1;                                                       \
   int MACRO_positive = 0;                                                     \
   int MACRO_dxy = 0, MACRO_do_xy;                                             \
+  CHECK_MAP_POS(ARG_start_x, ARG_start_y);                                    \
   while(MACRO_dxy <= (ARG_max_dist)) {                                        \
     for (MACRO_do_xy = -MACRO_dxy; MACRO_do_xy <= MACRO_dxy; MACRO_do_xy++) { \
       if (MACRO_xcycle) {                                                     \
@@ -411,6 +416,7 @@
 {                                                                             \
   int SI_x_itr, SI_y_itr;                                                     \
   int SI_x_itr1, SI_y_itr1;                                                   \
+  CHECK_MAP_POS(SI_center_x, SI_center_y);                                    \
   for (SI_y_itr1 = (SI_center_y) - (radius);                                  \
        SI_y_itr1 <= (SI_center_y) + (radius); SI_y_itr1++) {                  \
     for (SI_x_itr1 = (SI_center_x) - (radius);                                \
@@ -429,6 +435,7 @@
 {                                                                             \
   int RI_x_itr, RI_y_itr;                                                     \
   int RI_x_itr1, RI_y_itr1;                                                   \
+  CHECK_MAP_POS(RI_center_x, RI_center_y);                                    \
   for (RI_y_itr1 = RI_center_y - 1;                                           \
        RI_y_itr1 <= RI_center_y + 1; RI_y_itr1++) {                           \
     for (RI_x_itr1 = RI_center_x - 1;                                         \
@@ -453,7 +460,7 @@
   int x_itr, y_itr, dir_itr, MACRO_border;                                    \
   int MACRO_center_x = (center_x);                                            \
   int MACRO_center_y = (center_y);                                            \
-  assert(is_normal_map_pos(MACRO_center_x, MACRO_center_y));                  \
+  CHECK_MAP_POS(MACRO_center_x, MACRO_center_y);                              \
   MACRO_border = IS_BORDER_MAP_POS(MACRO_center_x, MACRO_center_y);           \
   for (dir_itr = 0; dir_itr < 8; dir_itr++) {                                 \
     DIRSTEP(x_itr, y_itr, dir_itr);                                           \
@@ -529,6 +536,7 @@
 {                                                      \
   int IAC_i;                                           \
   int IAC_x, IAC_y;                                    \
+  CHECK_MAP_POS(x, y);                                 \
   for (IAC_i = 0; IAC_i < 4; IAC_i++) {                \
     switch (IAC_i) {                                   \
     case 0:                                            \
Index: server/maphand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/maphand.c,v
retrieving revision 1.88
diff -u -r1.88 maphand.c
--- server/maphand.c    2001/10/30 10:59:21     1.88
+++ server/maphand.c    2001/11/01 12:56:38
@@ -769,14 +769,11 @@
 ***************************************************************/
 int map_get_known_and_seen(int x, int y, struct player *pplayer)
 {
-  int is_real = normalize_map_pos(&x, &y);
-  int playerid=pplayer->player_no;
-  int offset = map_inx(x, y);
+  int playerid = pplayer->player_no;
+  int offset   = map_inx(x, y);
 
-  assert(is_real);
-
-  return ((map.tiles + offset)->known) & (1u << playerid) &&
-      (pplayer->private_map + offset)->seen;
+  return ((map.tiles + offset)->known) & (1u << playerid)
+    && (pplayer->private_map + offset)->seen;
 }
 
 /***************************************************************
Index: server/settlers.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/settlers.c,v
retrieving revision 1.112
diff -u -r1.112 settlers.c
--- server/settlers.c   2001/10/14 21:02:17     1.112
+++ server/settlers.c   2001/11/01 12:56:39
@@ -405,8 +405,6 @@
 **************************************************************************/
 static int is_already_assigned(struct unit *myunit, struct player *pplayer, 
int x, int y)
 {
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
   if (same_pos(myunit->x, myunit->y, x, y) ||
       same_pos(myunit->goto_dest_x, myunit->goto_dest_y, x, y)) {
 /* I'm still not sure this is exactly right -- Syela */
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.143
diff -u -r1.143 unittools.c
--- server/unittools.c  2001/10/15 13:42:52     1.143
+++ server/unittools.c  2001/11/01 12:56:42
@@ -1628,8 +1628,7 @@
   idex_register_unit(punit);
   punit->owner=pplayer->player_no;
 
-  assert(is_real_tile(x, y));
-  normalize_map_pos(&x, &y);
+  CHECK_MAP_POS(x, y);
   punit->x = x;
   punit->y = y;
 
@@ -2895,7 +2894,7 @@
   struct tile *psrctile = map_get_tile(src_x, src_y);
   struct tile *pdesttile = map_get_tile(dest_x, dest_y);
 
-  assert(is_normal_map_pos(dest_x, dest_y));
+  CHECK_MAP_POS(dest_x, dest_y);
 
   conn_list_do_buffer(&pplayer->connections);
 

[Prev in Thread] Current Thread [Next in Thread]