Complete.Org: Mailing Lists: Archives: freeciv-dev: May 2002:
[Freeciv-Dev] Re: Wonder obsolesence across games (PR#1434)
Home

[Freeciv-Dev] Re: Wonder obsolesence across games (PR#1434)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: rf13@xxxxxxxxxxxxxxxxxxxxxx
Cc: Mike Kaufman <kaufman@xxxxxxxxxxxxxxxxxxxxxx>, freeciv-dev@xxxxxxxxxxx, bugs@xxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: Wonder obsolesence across games (PR#1434)
From: Ben Webb <ben@xxxxxxxxxxxxxxxxxxxxxx>
Date: Thu, 16 May 2002 11:19:00 +0100

On Wed, May 15, 2002 at 10:09:45PM +0200, Raimar Falke wrote:
> Here is a second fix. There are no additional memory leaks by calling
> game_init.

> -  geff_vector_init(&game.effects);
> -  ceff_vector_init(&game.destroyed_effects);
> +  geff_vector_reinit(&game.effects);
> +  ceff_vector_reinit(&game.destroyed_effects);

        This is bad. The first time you call game_init you are calling
_reinit on something that hasn't yet been initialized! (Your later patch
obviously suffers from exactly the same problem.) Plus, you're still not
dealing with player.effects properly (well, at all). The _init function
is supposed to be used to, er, initialize your specvec type. If you
don't call it before using the type (which is what your patch is doing)
then you're going to get undefined behaviour. (It's only luck that is
saving you at the moment - these two types just happen to be globals, so
start off as NULL, which just happens with the _current_ implementation of
athing to be what ath_init sets them to.)

> +void SPECVEC_FOO(_vector_reinit) (SPECVEC_VECTOR * tthis)
> +{
> +  if (SPECVEC_FOO(_vector_size) (tthis)) {
> +    SPECVEC_FOO(_vector_free) (tthis);
> +  }
> +  SPECVEC_FOO(_vector_init) (tthis);
> +}
> +

        This function is redundant, as _vector_free already does this.
You need proper functions to free your types at game end, not nasty
hacks; the former will be able to deal with your vectors requiring
freeing of instance data. I have submitted patches to this thread
that address these issues already. You can still go and call game_init
if you really want to; it should just be redundant.

        Ben
-- 
ben@xxxxxxxxxxxxxxxxxxxxxx           http://bellatrix.pcl.ox.ac.uk/~ben/
"And do you know Carolina where the biscuits are soft and sweet?"


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