[Freeciv-Dev] Re: init_techs integrated patch (PR#999)

[Arien Malec <arien_malec@xxxxxxxxx>]

--- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
> > +    int *init_techs; /* Advances to assign to all players at game
> > +   start. A_LAST terminated.*/
> 
> Why is this dynamic and not init_techs[MAX_NUM_INIT_TECHS + 1]? Or
> should MAX_NUM_INIT_TECHS be named MAX_NUM_NATION_INIT_TECHS?

Probably the latter. 8 (the value for MAX_NUM_INIT_TECHS) is too low for
global_init_techs. E.g., if you wanted to test modern warfare, you would need:

writing & university (library & univ)
currency, banking, econ (marketplace, etc.)
conscription, metallurgy, tactics (rifle, etc.)
railroad, bridge building
etc. etc. etc.

I could statically allocate to MAX_NUM_TECH_LIST?

> > +
> > +  /* Init techs */
> > +  int     init_techs[MAX_NUM_INIT_TECHS + 1]; /* A_LAST terminated */
> 
> For easy grepping and easy distinction it would be nice if you rename
> one of init_techs. May one init_techs and one global_init_techs?

OK

> > +    techs = secfile_lookup_str_vec(file, &dim, "%s.init_techs", sec[i]);
> > +    if( dim > MAX_NUM_INIT_TECHS ) {
> > +      freelog(LOG_VERBOSE,
> > +              "Only %d initial techs can be given from %d"
> > +       "defined for nation %s",
> > +              MAX_NUM_INIT_TECHS, dim, pl->name_plural);
> > +    }
> > +    for(j = k = 0; j < dim; j++) {
> 
> Overrun. Either you limit dim to MAX_NUM_INIT_TECHS or bail out above.

Agreed.
 
> > 
> /**************************************************************************
> > +Lookup init techs from section file
> >
> +**************************************************************************/
> > +static int *secfile_lookup_init_techs (struct section_file *file)
> 
> This method contains the same? code as above. Please unify them.

It'll take some effort to unify them, but I'll do so. 

Arien

__________________________________________________
Do You Yahoo!?
NEW from Yahoo! GeoCities - quick and easy web site hosting, just $8.95/month.
http://geocities.yahoo.com/ps/info1