[Freeciv-Dev] Re: Changing interface for generate_warmap (PR#1108)

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

At 06:39 PM 01/12/13 +0000, Gregory Berkolaiko wrote:
> --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote: 
>
>> Your patch has code like
>> +  generate_warmap(NULL, x, y, NULL);
>> +  boatdist = get_warmap_sea(NULL, x, y);
>> 
>> if this code uses a non-global warmap (which is the goal) it has to be
>> changed to:
>> +  generate_warmap(&wm, x, y, NULL);
>> +  boatdist = get_warmap_sea(&wm, x, y);
>> 
>> If you change your patch so that the code looks like
>> +  pwm = generate_warmap(x, y, NULL, 1);
>> +  boatdist = get_warmap_sea(pwm, x, y);
>> 
>> that you only have to change the "1" to use a non-global warmap. The
>> access function calls (get_warmap_sea and get_warmap_land) will remain
>> unchanged.
>
>Aha, I undertand now.  One very good argument in favour of not doing it
>this way was given by Jason.  I can only add a stupid argument: if you
>allocate the memory yourself, you are more likely to remember to free it
>(btw you forgot that to use non-local warmap, you have to do more than
>change 0 to 1, you have to free pwm).

Actually, it is not stupid. Forcing the caller to allocate the warmap
and pass it in, keeps the allocation and deallocation at the same point
in the code, and the responsibility clearly assigned.

Raimar uses the same split memory handling pattern a lot, and by 
preference it seems. But it leads to all kinds of subtle coding rules 
and errors from violating them. The doc fix usually is at the wrong 
point in the code (i.e. buried in a canned routine), so people never 
even know they missed something.

Just look at this code and try to understand how one would have noticed
a free bug without understanding the internal implications like Gregory.

>G.
>
>__________________________________________________
>Do You Yahoo!?
>Everything you'll ever need on one web page
>from News and Sport to Email and Music Charts
>http://uk.my.yahoo.com

Cheers,
RossW
=====