[Freeciv-Dev] Re: (PR#6898) leaks in game_load / player_load

["Benoit Hudson" <bh@xxxxxxxxxxxxxxxxxxx>]

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

On Tue, Nov 18, 2003 at 12:40:49PM -0800, Mike Kaufman wrote:
>
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=6898 >
>
> On Tue, Nov 18, 2003 at 09:44:57AM -0800, Benoit Hudson wrote:
> >
> > <URL: http://rt.freeciv.org/Ticket/Display.html?id=6898 >
> >
> > game_load does allot_island_improvs and then calls player_load.  In
> > turn, player_load does player_init.  Finally, player_init clobbers an
> > array that allot_island_improvs allocated, resulting in a memory leak.
>
> hmm, that does seem bad. Can you think of a solution?

Possibly player_init should fc_realloc instead of fc_malloc -- but then
I'm worried that player_init might be called on an uninitialized struct
Player in which case that could be very bad.

Alternatively, don't call allot_island_improvs before player_load -- but
I'm not sure this is correct.

> > It seems like allot_island_improvs isn't necessary, but it's not clear
> > to me.
>
> yeah, it's sorta important. see
> common/improvement.c:improvement_redundant() and
> server/maphand.c:check_for_continent_change()

I miswrote: I meant not necessary in this context (just before calling
player_load).  I'm betting that the intented set of events is:
        - player_init
        - play the game
                => if a new continent was created, call
                        allot_island_improvs

If so, then not calling allot_island_improvs is the right thing, and the
patch would be mercifully small.

        -- Benoît