[Freeciv-Dev] (PR#4769) Re: Re: (PR#4761) civserver get_invention crash

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

Jason Short wrote:
> This is caused by the F_UNDISBANDABLE flag.  When such a unit cannot be 
> supported a unit of population is taken from the city instead.  Nobody 
> checks to see if this causes the city to disband.  Random memory 
> corruption can result.
> 
> The attached patch should fix this particular instance.  However I 
> suspect all callers of city_reduce_size should check the return value.

Of course there are no UNDISBANDABLE units with upkeep in the default 
ruleset, so it's a bug that we hit this code at all.  The logic in 
city_distribute_surplus_shields is faulty and will cause population loss 
any time you have to disband more than one unit from a city at a time.

Fix attached; this is in addition to the other (memory corruption) 
patch.  You can see the current code is faulty by adding in the 'assert' 
line from this patch all by itself: the assertion will trigger.

jason

Index: server/cityturn.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/cityturn.c,v
retrieving revision 1.218
diff -u -r1.218 cityturn.c
--- server/cityturn.c   2003/08/04 15:42:47     1.218
+++ server/cityturn.c   2003/08/06 21:53:09
@@ -911,10 +911,16 @@
        break;
       }
     } unit_list_iterate_end;
+  }
+
+  while (pcity->shield_surplus < 0) {
     /* Special case: F_UNDISBANDABLE. This nasty unit won't go so easily. It'd
-       rather make the citizens pay in blood for their failure to upkeep it! */
+     * rather make the citizens pay in blood for their failure to upkeep it!
+     * If we make it here all normal units are already disbanded, so only
+     * undisbandable ones remain. */
     unit_list_iterate(pcity->units_supported, punit) {
       if (utype_shield_cost(unit_type(punit), g) > 0) {
+       assert(unit_flag(punit, F_UNDISBANDABLE));
        notify_player_ex(pplayer, pcity->x, pcity->y, E_UNIT_LOST,
                         _("Game: Citizens in %s perish for their failure to "
                         "upkeep %s!"), pcity->name, unit_type(punit)->name);