[Freeciv-Dev] (PR#8975) cm clear_cache() called before build_cache3()

["James Canete" <use_less@xxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8975 >

> [jdorje - Sun Jun 13 20:57:10 2004]:
> 
> James Canete wrote:
> > 
> >>[jdorje - Sun Jun 13 16:33:42 2004]:
> >>
> >>
> >>But without the assert added in there is no NULL dereference.  The 
> >>pointer is NULL but it's never dereferenced.
> > 
> > Doesn't this line in clear_cache() dereference it?
> >       if (!cache3.results[i].combinations[j].is_valid) {
> > 
> > That's assuming MAX_FIELDS_USED and MAX_COMBINATIONS are both nonzero,
> > though.  MAX_COMBINATIONS is #define'd to 150, and MAX_FIELDS_USED is
> > #define'd to (CITY_TILES - 1).
> 
> CITY_TILES is 0 at this point.
> 
> (Which means MAX_COMBINATIONS is -1, which would probably give problems 
> if we actually tried to realloc cache3.results.  Of course you guys 
> wanted to remove the -1 from MAX_COMBINATIONS, so this wouldn't be a 
> problem.)
> 
> But this means that the above line should never be executed.
> 
> Can you get a crash without the assertion you added?  If so what is it?

The way I got a crash was with MAX_FIELDS_USED #define'd to
(CITY_TILES), which means that even if CITY_TILES is set to zero, it
goes through this loop once:
  for (i = 0; i < MAX_FIELDS_USED + 1; i++) {

I'm thinking all the "+ 1"s used with MAX_FIELDS_USED should probably be
removed if MAX_FIELDS_USED were set equal to CITY_TILES, then.

-James Canete