[Freeciv-Dev] Re: {gen,spec,sort}list stuff

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Thu, Aug 30, 2001 at 12:31:35PM -0400, Justin Moore wrote:
> 
> > > I've started to notice some inconsistencies in games using the same random
> > > seeds (map and game) between games that use the old genlist and ones that
> > > use my genlist.  Yet I can't find any problems in my implementation of
> > > genlist (yes, I realize that by saying this I'm opening myself up to
> > > someone finding incredibly obvious bugs ... which is almost the point).
> > > I'd appreciate any extra eyes looking at the code.
> >
> > Is it possible to have both lists in parallel and compare their results.
> 
>    Yes, I'll get back to everyone on this.
> 
>    In some more recent profiles I've been able to cut down the number of
> old iterations by about 66% for a speed savings of around 5%.
> 
> > >    There are currently two kinds of list iteration macros: one (STABLE_*)
> > > for lists where the number of elements doesn't change and another (REAL_*)
> > > for those that do (REAL_ because that's what most use ATM).  The REAL_*
> > > macros use the iterators, which are painfully slow but are very tolerant
> > > of one or more elements being removed from underneath them.
> >
> > I'm not happy with the name REAL_.
> 
>    SAFE_?
> 
> > Please people test it. Looking at it I think that you should be more
> > restrictive with your arguments:
> >  - pgenlist->capacity should be set for every list and yet you test
> >  for it in genlist_resize
> 
>    I had, at one point, thought about reclaiming memory if the current
> list size went below 1/4 of the allocated size.  The check was meant to be
> a safeguard against some weird math problem that may have happened
> somewhere along the line, but it's really not necessary.
> 
> >  - is this mess with the current genlist_sort and NULL needed? It is
> >  used.
> 
>    Could you be more specific?  I'm just replicating the external behavior
> of genlist_sort when given a NULL compar pointer: nothing is done to the
> list.

The external behavior is stupid IMHO. It isn't used anywhere in the
code. This can be removed in the current code. No need to code such
no-used features in new versions.

> >  - which code unlinks none items which aren't in the list (assert,
> >  assert,...)

  Remove an element of the genlist with the specified user-data pointer
  given by 'punlink'.  If there is no such element, does nothing.
  If there are multiple such elements, removes the first one.

+  idx = genlist_find_index(pgenlist, punlink);
+  if(idx == -1)
+    return;

The current documentation and implementation allows it. But this can
be put on the caller. So only item in the list were allowed to be
unlinked. I would suspect that most callers unlink items from the
list.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Using only the operating-system that came with your computer is just
  like only playing the demo-disc that came with your CD-player."