[Freeciv-Dev] Re: Remove map_adjust_[xy] invocations from server (PR#1003)

[Gaute B Strokkenes <gs234@xxxxxxxxx>]

On Mon, 22 Oct 2001, hawk@xxxxxxxxxxxxxxxxxxxxxxx wrote:

> I agree to this solution for all positions are at some time used as
> arguments to map_get_tile and co.  The question is: what happends to
> the rest? 

> Either we manually go through the code and set
> assert(is_normal_map_pos()) (which is error prone and not robust)

Or we add the macro that we are going to stick inside map_inx().

I'm not sure why you think this is error prone and not robust; can you
elaborate?  It's certainly not any worse than "the big hammer"
mentioned below.

I think it is useful to take a step back and consider how likely such
mistakes are going to be.  Converting manual loops to _iterate macros
is fairly easy to do, and it is also easy to spot most of the other
mistakes that lead to this situation.

It would be a good idea to add those checks in places where it is
obvious that a check in map_inx() is not enough, such as same_pos().
I would not spend too much time and effort trying to make 100% sure
that we have found all the places where we might be able to spot such
a mistake).

> OR we ignore this case 

> OR we take out the big hammer and insert assert(is_normal_map_pos())
> into almost every function and disable it by default (because this
> will have a performance impact).

The idea is to disable it by default, and let people turn it back on
when they make changes to iteration macros etc. that need testing of
this sort.  Since that is a distinct minority of the time, we do not
need to care that much about how much time it takes.  I don't think it
is going to take that much more time than the current approach (always
check, always fix).



-- 
Big Gaute                               http://www.srcf.ucam.org/~gs234/
Why don't you ever enter any CONTESTS, Marvin??
 Don't you know your own ZIPCODE?