[Freeciv-Dev] Re: [PATCH] Small improvement at genlist and maphand

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Aug 20, 2001 at 04:32:11PM -0700, Trent Piepho wrote:
> On Mon, 20 Aug 2001, Raimar Falke wrote:
> > given id is still valid. A pointer will always be valid. So a new
> > field in every struct is needed to indicate invalid structs. Such a
> > flag will be set if a remove_unit packet is received for example.
> 
> But then the structure is freed, so who knows what the valid field will get
> changed to, since it's now in free memory.  And if you pass in a random
> pointer, who's to say that it won't have the right bytes to give it a proper
> valid field?

I haven't thought about this. Also some magic fields won't help here
since the freed memory can get reused for the same object type. One
solution is to don't free structs but there is no way to get this into
freeciv. The only other solution I found is doing the analog of a
reverse name lookup: "assert(find_unit_by_id(punit->id)==punit);" This
won't create an immediate performance boost but step after step such
asserts can be disabled.

from cvs annotate freeciv_hackers_guide.txt:

1.1          (freeciv  01-May-98): [Currently all of the major entities - 
units, cities, players, contains
1.1          (freeciv  01-May-98): an unique id. This id is really only 
required when a reference to an entity
1.1          (freeciv  01-May-98): is to be serialized(saved or distributed 
over the net). However in the
1.1          (freeciv  01-May-98): current code, the id is also used for 
comparing, looking up and in general
1.2          (freeciv  05-Aug-98): referencing entities. This results in a lot 
of mess and unnecessary duplicate
1.2          (freeciv  05-Aug-98): functions. Often when programming, one 
wonders if some function needs
1.2          (freeciv  05-Aug-98): the id or a pointer when referring to an 
entity. -PU]

So this is a long known problem and should be fixed.

So what people think about such a plan:
 - only pointers are passed around, no more ids
 - every id-to-pointer method has to be replaced by a call to a
 "ensure_valid_*" method/macro
 - step after step such "ensure_valid_*" are removed are replaced by a
 call to "lax_ensure_valid_*" which is either a noop or a call to
 "ensure_valid_*" depending on some define

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "That's fundamental game play!  My main enemy is *ALWAYS* fighting 
  a 4-front war.  I make sure of it!"
    -- Tony Stuckey, freeciv-dev