[Freeciv-Dev] Re: list cleaning

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

On Fri, Jan 23, 2004 at 06:17:04PM +0000, Per I. Mathisen wrote:
> On Fri, 23 Jan 2004, Raimar Falke wrote:
> > > > > > Can you please repeat the problems which the current code has?
> > > > >
> > > > > unit_list_iterate is unsafe
> > > >
> > > > Yes but I don't think it is a problem or bug. It is designed this way.
> > >
> > > It certainly causes bugs. One has to wonder if there are any unsafe
> > > uses of it.
> >
> > Will such bugs show quickly? AFAIK the answer is yes. IMHO the
> > cost/benefit ratio for a change is bad.
> 
> 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. Also this is not a problem which Freeciv code per se. It
is a bad idea for example in python to do:

for x in l:
  if cond(x):
    l.remove(x)

you either do

l2=[]
for x in l:
  if not cond(x):
    l2.append(x)
l=l2

or

l=filter(lambda x: not cond(x), l)

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.

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

??

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  Two OS engineers facing a petri net chart:
        "dead lock in four moves!"