[Freeciv-Dev] Re: Profile.

[Gaute B Strokkenes <gs234@xxxxxxxxx>]

On Wed, 26 Sep 2001, hawk@xxxxxxxxxxxxxxxxxxxxxxx wrote:
> 
> There is hope! After the removal of the DIR_DX array I made some
> changes to catch any un-real map positions. There were none. So I
> changed it to catch any non-normal(ized) map accesses. They came
> from lines such as:
> 
>   enum tile_special_type spec_t=map_get_special(pcity->x+x-2, pcity->y+y-2);
>   enum tile_terrain_type tile_t=map_get_terrain(pcity->x+x-2, pcity->y+y-2);
> 
> The proposed patch would get rid of these.

I think that's a good idea.

> Although I haven't checked I'm sure that there are none or only a
> very few non-normal(ized) map accesses left. So we can in theory
> remove a lot of the is_real_tile and normalize_map_pos calls in
> map_get_tile and co. However for the ability to check this in the
> long run I would like to propose the following:
> 
> #define CHECK_MAP_POS(x,y) do{\
>     int dx = x, dy = y; \
>     assert(normalize_map_pos(&dx, &dy)); \
>     assert(x == dx && y == dy); \
>   } while(0)

Are you aware of check_coords() ?

> So map_get_tile would be like:
> 
> struct tile *map_get_tile(int x, int y)
> {
>   CHECK_MAP_POS(x,y);
> 
>   return map.tiles + map_inx(x, y);
> }
> 
> CHECK_MAP_POS can now check for 
>  - un-real map position or for
>  - non-normal(ized) map positions or
>  - can be a noop

This is very similar to what I've been thinking about.  However, I
think we should embedded this in map_inx() instead.  I think that
would be a lot less intrusive and probably more thorough.

#ifndef NDEBUG

#define map_inx(x,y) \
  (SANE_POS(x,y) ? (x) + (y) * map.xsize : abort())

#else

#define map_inx(x,y) \
  ((x) + (y) * map.ysize)

#endif /* ! NDEBUG */

> This can be tied to NDEBUG or something other.

Yup.  I would propose to move it to DEBUG after a while as we gain
confidence that we don't have this sort of bug anymore.

-- 
Big Gaute                               http://www.srcf.ucam.org/~gs234/
YOW!!  The land of the rising SONY!!