Complete.Org: Mailing Lists: Archives: freeciv-dev: August 2001:
[Freeciv-Dev] Re: [PATCH] Re: {gen,spec,sort}list stuff

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

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: Freeciv Developers <freeciv-dev@xxxxxxxxxxx>
Subject: [Freeciv-Dev] Re: [PATCH] Re: {gen,spec,sort}list stuff
From: Justin Moore <justin@xxxxxxxxxxx>
Date: Fri, 31 Aug 2001 14:25:18 -0400 (EDT)

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

   I just ran a test with both lists running in parallel using bunches of
asserts around the wrapper calls to genlist_*.  There was no difference in
the contents of the two lists throughout the game, so I have no idea why
one would obtain slightly different results from the other when run
separately.  In this light, I'm submitting my genlist.[ch] files as a
patch to the current CVS tree.

> > > I'm not happy with the name REAL_.
> >
> >    SAFE_?

   For lack of comments, there are the new names.

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

The code in question:

 void genlist_sort(struct genlist *pgenlist,
                  int (*compar)(const void *, const void *))
   if(compar == NULL)

   qsort(pgenlist->data, pgenlist->nelements, sizeof(void*), compar);

   So what should I do if the compar is NULL?

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

   I've put an assert(idx != -1) after the function call.  This way we'll
know when something's wrong in a development version but it won't break in
a release.

   If this is accepted, I have a slew of patches (all small :)) that I can
throw out to start moving from the slower iterators to the newer ones.


Department of Computer Science, Duke University, Durham, NC 27708-0129
Email:  justin@xxxxxxxxxxx

Attachment: genlist-Aug-31-cvs.patch.gz
Description: new genlist

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