Complete.Org: Mailing Lists: Archives: freeciv-dev: January 2004:
[Freeciv-Dev] Re: list cleaning
Home

[Freeciv-Dev] Re: list cleaning

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: freeciv-dev@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: list cleaning
From: "Per I. Mathisen" <per@xxxxxxxxxxx>
Date: Mon, 26 Jan 2004 10:20:04 +0000 (GMT)

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.

> 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.

> > 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.

  - Per



[Prev in Thread] Current Thread [Next in Thread]