[Freeciv-Dev] Re: (PR#8521) sha.c crashes

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

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8521 >

On Fri, Apr 16, 2004 at 12:07:23PM -0700, Jason Short wrote:
> 
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=8521 >
> 
> Raimar Falke wrote:
> > <URL: http://rt.freeciv.org/Ticket/Display.html?id=8521 >
> > 
> > On Thu, Apr 15, 2004 at 02:56:54PM -0700, Jason Short wrote:
> > 
> >><URL: http://rt.freeciv.org/Ticket/Display.html?id=8521 >
> >>
> >>When I play for any amount of time there's a segfault in sha.c. 
> >>Something like this:
> >>
> >>#0  sha_unit_new (id=152) at sha.c:69
> >>69    struct unit *pold_unit = 
> >>create_unit_virtual(get_player(punit->owner),(gdb) bt
> >>#0  sha_unit_new (id=152) at sha.c:69
> >>#1  0x08120c57 in execute_call (call=0x82d93d0) at agents.c:213
> >>#2  0x08120cee in call_handle_methods () at agents.c:251
> >>#3  0x08120d77 in thaw () at agents.c:287
> >>
> >>Please fix.
> > 
> > 
> > Of course the unit doesn't have to exists anymore when sha_unit_new is
> > called.
> > 
> > I also added unit_clone and unit_destroy.
> 
> I don't think the logic of
> 
>    punit = create_unit_virtual(...);
>    punit2 = unit_clone(punit);
>    destroy_unit_virtual(punit);
>    destroy_unit(punit2);
> 
> is good at all.  If punit2 is a virtual unit (which it obviously is) it 
> should be destroyed by destroy_unit_virtual.
> 
> Which leads me to conclude that destroy_unit should be removed and 
> unit_clone should be clone_unit_virtual.

> A "virtual" unit is the part of the unit allocation that is common 
> between client and server.  We don't need two concepts for this.

Uhhh. Care to cite something? AFAIK virtual means that the unit
doesn't exists (a virtual unit in the client is unknown to the server,
a virtual unit in the server-ai is unknown to the server,...). The
original reason for it was that some function's wanted "struct unit *"
so so you created a virtual unit to pass into them. It was first
introduced by Gregory in the air goto patches:

+/**********************************************************
+ * Create a virtual unit to use in build want estimation
+ *********************************************************/
+struct unit *create_unit_virtual(struct player *pplayer, int x, int y,
+                                Unit_Type_id type, bool make_veteran)

So we should drop the _virtual and just name it
"unit_create". "unit_destroy" can then be used for both "unit_create"
and "unit_clone". unittools.c:create_unit should be renamed to
unittools.c:server_create_unit or similar to denote that more work is
done here.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "I haven't lost my mind - it's backed up on tape somewhere."