[Freeciv-Dev] Re: CVS core

[Jason Dorje Short <jshort@xxxxxxxxxxxxx>]

Paul Zastoupil wrote:
> 
> I was profiling today's cvs version since Gaute commited Jason's patch.
> 
> civserver: gotohand.c:310: really_generate_warmap: Assertion `0 <= y && y <= 
> map.ysize && 0 <= x && x <= map.xsize' failed.
> Aborted (core dumped)

This is because the macro Gaute committed is buggy in three ways:

- The check on y_itr for the border case is wrong.  This is what's
causing the problem.

- There's a potential problem with the use of the center_x/center_y
arguments - if an expression is passed in, it will be evaluated multiple
times (or possibly used incorrectly).

- The assertion is wrong - it should not allow equality in the case of
the upper bounds.

Patch attached.

jason

Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.81
diff -u -r1.81 map.h
--- common/map.h        2001/07/30 22:59:00     1.81
+++ common/map.h        2001/07/31 01:19:52
@@ -370,17 +370,19 @@
 #define adjc_dir_iterate(center_x, center_y, x_itr, y_itr, dir_itr)           \
 {                                                                             \
   int x_itr, y_itr, dir_itr, MACRO_border;                                    \
-  assert(0 <= center_y && center_y <= map.ysize                               \
-         && 0 <= center_x && center_x <= map.xsize);                          \
-  MACRO_border = (center_y == 0                                               \
-                  || center_x == 0                                            \
-                  || center_y == map.ysize-1                                  \
-                  || center_x == map.xsize-1);                                \
+  int MACRO_center_x = (center_x);                                            \
+  int MACRO_center_y = (center_y);                                            \
+  assert(0 <= MACRO_center_y && MACRO_center_y < map.ysize                    \
+         && 0 <= MACRO_center_x && MACRO_center_x < map.xsize);               \
+  MACRO_border = (MACRO_center_y == 0                                         \
+                  || MACRO_center_x == 0                                      \
+                  || MACRO_center_y == map.ysize-1                            \
+                  || MACRO_center_x == map.xsize-1);                          \
   for (dir_itr = 0; dir_itr < 8; dir_itr++) {                                 \
-    y_itr = center_y + DIR_DY[dir_itr];                                       \
-    x_itr = center_x + DIR_DX[dir_itr];                                       \
+    y_itr = MACRO_center_y + DIR_DY[dir_itr];                                 \
+    x_itr = MACRO_center_x + DIR_DX[dir_itr];                                 \
     if (MACRO_border) {                                                       \
-      if (y_itr < 0 || y_itr > map.ysize)                                     \
+      if (y_itr < 0 || y_itr >= map.ysize)                                    \
         continue;                                                             \
       if (x_itr < 0)                                                          \
         x_itr += map.xsize;                                                   \