[Freeciv-Dev] Re: Potential segfaults in advdomestic.c (PR#1599)

[Gregory Berkolaiko <Gregory.Berkolaiko@xxxxxxxxxxxx>]

The attached patch should fix the problems outlined below.
The following changes were made:

1. An assert in get_unit_type will insure the type_id is valid and no 
overflow occurs.
2. The code dealing with caravans helping wonders separated from 
domestic_advisor_choose_build into a separate function 
ai_choose_help_wonder.
3. No help_wonder code is executed if there are no such units available in 
the ruleset (will be compatible with civIII).
4. Make sure get_unit_type is called with a valid caravan type.

Somewhat surprisingly, the behaviour isn't changed (testing savegames).  
This is because (3) isn't relevant for the default ruleset and (4) is 
changing the behaviour only when a wonder is built but caravans are not 
yet available -- a rare situation.

Review, criticise, apply.

G.


On Wed, 19 Jun 2002, Gregory Berkolaiko wrote:

> I started looking at it because of (PR#1580).
> 
> While I was unable to reproduce or explain the crash described below, I
> noticed a place where the code is not robust wrt ruleset changes.  The
> line
> http://www.freeciv.org/lxr/source/ai/advdomestic.c?v=cvs#L764
> 
> unit_type = get_role_unit(F_HELP_WONDER, 0);
> 
> presumes that there is a unit which can help build wonder.  If there is no 
> such unit in the ruleset, an assert in get_role_unit will be triggered.
> Tested, it happens indeed.
> 
> There is at least another bug around here.  Line 744 calls get_unit_type
> but the argument is not guaranteed to be below U_LAST, which is
> essentially a segfault.  I think placing an assert in the function
> get_unit_type (unittype.c:84) should reveal a multitude of such
> overflowing calls.
> 
> G.
> 
> 
> On Sun, 16 Jun 2002 meusel@xxxxxxxxx wrote:
> 
> > Full_Name: Erik Meusel
> > Version: 1.12.0
> > Distribution: Debian binary
> > Client: Gtk+
> > OS: Debian GNU/Linux 3.0
> > Submission from: (NULL) (212.172.112.14)
> > 
> > 
> > Hi,
> > 
> > today I played freeciv with eight civilizations.
> > I was the german emperor "Bismarck". I pressed return
> > to end the current year. The last thing I saw was that
> > my capital "Berlin" was building a stone thrower (don't really
> > know the english word for it), then the connection to the
> > server stopped and the output was:
> > 
> > > civserver: ../../common/unittype.c:509: get_role_unit:
> > Zusicherung Лindex>=0 && index<n_with_role[role]Ћ nicht erfќllt.
> > Abgebrochen
> > 
> > Which means, that the assertion failed.
> > 
> > I'm sorry, but there's no backtrace and nothing similar. And I
> > don't know how to reproduce it. ;)
> > 
> > 
> > mfg, Erik
> > 
> > 
> > 
> > 
> 
> 
> 
> 
>

ai_help_wonder.diff
Description: Text document