[Freeciv-Dev] Re: list cleaning

[Raimar Falke <i-freeciv-lists@xxxxxxxxxxxxx>]

On Mon, Jan 26, 2004 at 10:20:04AM +0000, Per I. Mathisen wrote:
> On Mon, 26 Jan 2004, Raimar Falke wrote:
> > > By experience, such bugs do _not_ show quickly (errors may depend on list
> > > order and rare nested calls) and are not easy to track (random memory
> > > corruption).I fixed umphteen bugs related to unit_list_iterate after the
> > > allied transport patch and AI diplomacy went into cvs. Took quite a while
> > > before all bugs (we know of) were found and fixed.
> >
> > I still find it easier and more efficient to just write a big warning
> >
> > YOU SHALL NOT REMOVE ITEMS FROM A LIST OVER WHICH YOU ITERATE
> >
> > into Hacking.
> 
> Easier?? This means rewriting unit handling in the server. Probably other
> list handling as well.
> 
> Nested iterators are used all over the place, and not just in one place.
> You have eg an interator that calls procedures that have their own
> iterators that may or may not decide to remove units in the list.
> 
> The problem is that there are so many rules that imply a chance of
> removing a unit (can't move it off terraformed terrain? disband it!) that
> this rule cannot be enforced without a significant rewrite.

Note the above rule is already in effect. You make it sound like the
current server is buggy as hell. I don't see evidence for this.

> > Also this is not a problem which Freeciv code per se.
> ...
> > The reason why we have these problems in freeciv code is that we don't
> > separate between iterating over a list and filtering a list. Doing
> > these two tegether is a bad idea in the first place.
> 
> Interestingly, a few years ago I wrote a mechanism for separating the two,
> by delaying actions that would cause deletions, which I called triggers.
> You flat out rejected this patch as unnecessary.

Yes. Because the delayed effect of removing the unit is too far from
the cause. This is also a large problem I have with your current
lazy-delete patch. It just doesn't feel safe. With an early
destruction you can for example catch access to this now dead memory
with valgrind. With the lazy approach you can't.

> > > I also think it is a good thing in principle to add a *_free call whenever
> > > we have a *_init call to a code module.
> >
> > ??
> 
> If we have, eg, a nations_init(), we should also have a nations_done(),
> just for compleness, so that the day someone needs to add a cleanup
> operation in the latter, he does not have to second-guess the original
> author on where it needs to be put.

I agree but the only connection I see with the current issue is that
you want to place the lazy-delete function in the *_free() function?!

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 checking for the vaidity of the Maxwell laws on this machine... ok
 checking if e=mc^2... ok
 checking if we can safely swap on /dev/fd0... yes
    -- kvirc 2.0.0's configure