[Freeciv-Dev] "division by zero" error during compilation (PR#1954)

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

If I compile FreeCiv with gcc3 and -Werror, I get

gcc -DHAVE_CONFIG_H -I. -I. -I.. -I./include -I../common -I../intl 
-I./agents     -g -O3 -Wpointer-arith -Wcast-align -Wmissing-prototypes 
-Wmissing-declarations -Werror -g -Wall -c `test -f tilespec.c || echo 
'./'`tilespec.c
cc1: warnings being treated as errors
tilespec.c: In function `fill_tile_sprite_array_iso':
tilespec.c:1269: warning: division by zero
tilespec.c:1271: warning: division by zero
make[3]: *** [tilespec.o] Error 1

this has also been reported as a problem (not sure if it's an error or 
warning) under (IIRC) sun cc.
The "problem" is in the following code in tilespec.c:

  int array_index = ((ttype_near[DIR_CCW(dirs[i])] != T_OCEAN ? 1 : 0)
                      + (ttype_near[dirs[i]] != T_OCEAN ? 2 : 0)
                      + (ttype_near[DIR_CW(dirs[i])] != T_OCEAN ? 4 :
                          0));

because DIR_CW and DIR_CCW have a possibile division by zero (if the 
given direction is invalid, a division by zero is done to force an abort).
The simplest solution is to change the implementation of DIR_[C]CW so 
that the (dir)/0 part becomes (assert(0),dir).  This has a secondary 
advantage in that it won't crash the code when compiled with NDEBUG 
(although the behavior of the program may become undefined afterwards). 
 However, it is possible that some compilers might not like this 
construct either (I have no problems with gcc3).
One alternative would be to just drop the assert() part completely; 
however, I like checks like these and would be sorry to see that happen. 
  Another would be to replace assert() with abort(); this would 
preserve the current behavior (which, as I said, I think is a bad idea). 
 Yet another would be to change the calling code to avoid this sort of 
construct, but that's just a hack - and will make the already ugly 
tilespec code even less pretty.
Patch attached.

jason

Index: common/map.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
retrieving revision 1.129
diff -u -r1.129 map.h
--- common/map.h        2002/08/07 11:21:47     1.129
+++ common/map.h        2002/08/21 17:55:29
@@ -248,7 +248,7 @@
        ((dir)==DIR8_NORTHEAST ? DIR8_EAST : \
         ((dir)==DIR8_SOUTHWEST ? DIR8_WEST : \
          ((dir)==DIR8_SOUTHEAST ? DIR8_SOUTH : \
-         (dir)/0))))))))
+         (assert(FALSE), dir)))))))))
 
 /*
  * Returns the next direction counter-clock-wise
@@ -262,7 +262,7 @@
        ((dir)==DIR8_NORTHEAST ? DIR8_NORTH : \
         ((dir)==DIR8_SOUTHWEST ? DIR8_SOUTH : \
          ((dir)==DIR8_SOUTHEAST ? DIR8_EAST : \
-         (dir)/0))))))))
+         (assert(FALSE), dir)))))))))
 
 struct city *map_get_city(int x, int y);
 void map_set_city(int x, int y, struct city *pcity);