Re: [Freeciv-Dev] (PR#38323) Crash when initial techs contain unreachable tech
http://bugs.freeciv.org/Ticket/Display.html?id=38323 > On 04/08/07, Marko Lindqvist <[EMAIL PROTECTED]> wrote: > > On 17/03/07, Marko Lindqvist <[EMAIL PROTECTED]> wrote: > > > > Nation does not have root_req for its initial tech. At least ruleset > > sanity checking is needed so that game gives sensible error message to > > ruleset author. > > I'm going to remove assert from S2_1 and commit full sanity checking > patch to trunk. Looking that code more closely, I now think that S2_1 needs that sanity checking part also. Who knows what effects (causing unnecessary bughunting) it will cause, if we let game to start with such invalid rulesets. - ML ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#38323) Crash when initial techs contain unreachable tech
http://bugs.freeciv.org/Ticket/Display.html?id=38323 >
On 17/03/07, Marko Lindqvist <[EMAIL PROTECTED]> wrote:
>
> On 3/17/07, Marko Lindqvist <[EMAIL PROTECTED]> wrote:
> >
> > Nation does not have root_req for its initial tech. At least ruleset
> > sanity checking is needed so that game gives sensible error message to
> > ruleset author.
>
> Sanity checking part of this ticket.
Updated against svn
- ML
diff -Nurd -X.diff_ignore freeciv/server/ruleset.c freeciv/server/ruleset.c
--- freeciv/server/ruleset.c 2007-08-03 22:00:33.0 +0300
+++ freeciv/server/ruleset.c 2007-08-04 01:47:45.0 +0300
@@ -124,6 +124,10 @@
static void send_ruleset_cities(struct conn_list *dest);
static void send_ruleset_game(struct conn_list *dest);
+static bool nation_has_initial_tech(struct nation_type *pnation,
+Tech_type_id tech);
+static bool sanity_check_ruleset_data(void);
+
/**
datafilename() wrapper: tries to match in two ways.
Returns NULL on failure, the (statically allocated) filename on success.
@@ -3509,6 +3513,8 @@
load_ruleset_effects(&effectfile);
load_ruleset_game();
+ sanity_check_ruleset_data();
+
precalc_tech_data();
script_free();
@@ -3552,3 +3558,92 @@
lsend_packet_thaw_hint(dest);
conn_list_do_unbuffer(dest);
}
+
+/**
+ Does nation have tech initially?
+**/
+static bool nation_has_initial_tech(struct nation_type *pnation,
+Tech_type_id tech)
+{
+ int i;
+
+ /* See if it's given as global init tech */
+ for (i = 0;
+ i < MAX_NUM_TECH_LIST && game.rgame.global_init_techs[i] != A_LAST;
+ i++) {
+if (game.rgame.global_init_techs[i] == tech) {
+ return TRUE;
+}
+ }
+
+ /* See if it's given as national init tech */
+ for (i = 0;
+ i < MAX_NUM_TECH_LIST && pnation->init_techs[i] != A_LAST;
+ i++) {
+if (pnation->init_techs[i] == tech) {
+ return TRUE;
+}
+ }
+
+ return FALSE;
+}
+
+/**
+ Some more sanity checking once all rulesets are loaded. These check
+ for some cross-referencing which was impossible to do while only one
+ party was loaded in load_ruleset_xxx()
+
+ Returns TRUE if everything ok.
+**/
+static bool sanity_check_ruleset_data(void)
+{
+ /* Check that all players can have their initial techs */
+ nations_iterate(pnation) {
+int i;
+
+/* Check global initial techs */
+for (i = 0;
+ i < MAX_NUM_TECH_LIST && game.rgame.global_init_techs[i] != A_LAST;
+ i++) {
+ Tech_type_id tech = game.rgame.global_init_techs[i];
+ if (!tech_exists(tech)) {
+freelog(LOG_FATAL, "Tech %s does not exist, but is initial "
+ "tech for everyone.",
+advance_rule_name(tech));
+exit(EXIT_FAILURE);
+ }
+ if (advances[tech].root_req != A_NONE
+ && !nation_has_initial_tech(pnation, advances[tech].root_req)) {
+/* Nation has no root_req for tech */
+freelog(LOG_FATAL, "Tech %s is initial for everyone, but %s has "
+ "no root_req for it.",
+advance_rule_name(tech),
+nation_rule_name(pnation));
+exit(EXIT_FAILURE);
+ }
+}
+
+/* Check national initial techs */
+for (i = 0;
+ i < MAX_NUM_TECH_LIST && pnation->init_techs[i] != A_LAST;
+ i++) {
+ Tech_type_id tech = pnation->init_techs[i];
+ if (!tech_exists(tech)) {
+freelog(LOG_FATAL, "Tech %s does not exist, but is tech for %s.",
+advance_rule_name(tech), nation_rule_name(pnation));
+exit(EXIT_FAILURE);
+ }
+ if (advances[tech].root_req != A_NONE
+ && !nation_has_initial_tech(pnation, advances[tech].root_req)) {
+/* Nation has no root_req for tech */
+freelog(LOG_FATAL, "Tech %s is initial for %s, but they have "
+ "no root_req for it.",
+advance_rule_name(tech),
+nation_rule_name(pnation));
+exit(EXIT_FAILURE);
+ }
+}
+ } players_iterate_end;
+
+ return TRUE;
+}
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#38323) Crash when initial techs contain unreachable tech
http://bugs.freeciv.org/Ticket/Display.html?id=38323 > On 17/03/07, Marko Lindqvist <[EMAIL PROTECTED]> wrote: > > Nation does not have root_req for its initial tech. At least ruleset > sanity checking is needed so that game gives sensible error message to > ruleset author. This is not trivial as we need to check if *nations* > are legal, but tech_is_available() works on *players*. > It also seems to me that this crash might happen if initial root_req > is simply processed later than than tech depending on it. This ticket has been closed, but only related commit I can find is assert removal from trunk. I'm going to remove assert from S2_1 and commit full sanity checking patch to trunk. - ML ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#38323) Crash when initial techs contain unreachable tech
http://bugs.freeciv.org/Ticket/Display.html?id=38323 > On 3/17/07, Marko Lindqvist <[EMAIL PROTECTED]> wrote: > It also seems to me that this crash might happen if initial root_req > is simply processed later than than tech depending on it. This patch simply removes assert(). - ML diff -Nurd -X.diff_ignore freeciv/server/techtools.c freeciv/server/techtools.c --- freeciv/server/techtools.c 2007-03-05 19:13:46.0 +0200 +++ freeciv/server/techtools.c 2007-03-17 19:21:16.0 +0200 @@ -275,7 +275,9 @@ assert((tech_exists(tech_found) && get_invention(plr, tech_found) != TECH_KNOWN) || tech_found == A_FUTURE); - assert(tech_is_available(plr, tech_found) || tech_found == A_FUTURE); + /* It is possible that !tech_is_available() simply because we are only + * setting up initial techs and root_req is one of them. + * assert(tech_is_available(plr, tech_found) || tech_found == A_FUTURE); */ /* got_tech allows us to change research without applying techpenalty * (without loosing bulbs) */ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#38323) Crash when initial techs contain unreachable tech
http://bugs.freeciv.org/Ticket/Display.html?id=38323 >
On 3/17/07, Marko Lindqvist <[EMAIL PROTECTED]> wrote:
>
> Nation does not have root_req for its initial tech. At least ruleset
> sanity checking is needed so that game gives sensible error message to
> ruleset author.
Sanity checking part of this ticket.
- ML
diff -Nurd -X.diff_ignore freeciv/server/ruleset.c freeciv/server/ruleset.c
--- freeciv/server/ruleset.c 2007-03-05 19:13:46.0 +0200
+++ freeciv/server/ruleset.c 2007-03-17 18:43:50.0 +0200
@@ -103,6 +103,10 @@
static void send_ruleset_cities(struct conn_list *dest);
static void send_ruleset_game(struct conn_list *dest);
+static bool nation_has_initial_tech(struct nation_type *pnation,
+Tech_type_id tech);
+static bool sanity_check_ruleset_data(void);
+
/**
datafilename() wrapper: tries to match in two ways.
Returns NULL on failure, the (statically allocated) filename on success.
@@ -3073,6 +3077,9 @@
load_ruleset_nations(&nationfile);
load_ruleset_effects(&effectfile);
load_ruleset_game();
+
+ sanity_check_ruleset_data();
+
translate_data_names();
precalc_tech_data();
@@ -3113,3 +3120,92 @@
lsend_packet_thaw_hint(dest);
conn_list_do_unbuffer(dest);
}
+
+/**
+ Does nation have tech initially?
+**/
+static bool nation_has_initial_tech(struct nation_type *pnation,
+Tech_type_id tech)
+{
+ int i;
+
+ /* See if it's given as global init tech */
+ for (i = 0;
+ i < MAX_NUM_TECH_LIST && game.rgame.global_init_techs[i] != A_LAST;
+ i++) {
+if (game.rgame.global_init_techs[i] == tech) {
+ return TRUE;
+}
+ }
+
+ /* See if it's given as national init tech */
+ for (i = 0;
+ i < MAX_NUM_TECH_LIST && pnation->init_techs[i] != A_LAST;
+ i++) {
+if (pnation->init_techs[i] == tech) {
+ return TRUE;
+}
+ }
+
+ return FALSE;
+}
+
+/**
+ Some more sanity checking once all rulesets are loaded. These check
+ for some cross-referencing which was impossible to do while only one
+ party was loaded in load_ruleset_xxx()
+
+ Returns TRUE if everything ok.
+**/
+static bool sanity_check_ruleset_data(void)
+{
+ /* Check that all players can have their initial techs */
+ nations_iterate(pnation) {
+int i;
+
+/* Check global initial techs */
+for (i = 0;
+ i < MAX_NUM_TECH_LIST && game.rgame.global_init_techs[i] != A_LAST;
+ i++) {
+ Tech_type_id tech = game.rgame.global_init_techs[i];
+ if (!tech_exists(tech)) {
+freelog(LOG_FATAL, _("Tech %s does not exist, but is initial "
+ "tech for everyone."),
+get_tech_name(NULL, tech));
+exit(EXIT_FAILURE);
+ }
+ if (advances[tech].root_req != A_NONE
+ && !nation_has_initial_tech(pnation, advances[tech].root_req)) {
+/* Nation has no root_req for tech */
+freelog(LOG_FATAL, _("Tech %s is initial for everyone, but %s has "
+ "no root_req for it."),
+get_tech_name(NULL, tech),
+pnation->name);
+exit(EXIT_FAILURE);
+ }
+}
+
+/* Check national initial techs */
+for (i = 0;
+ i < MAX_NUM_TECH_LIST && pnation->init_techs[i] != A_LAST;
+ i++) {
+ Tech_type_id tech = pnation->init_techs[i];
+ if (!tech_exists(tech)) {
+freelog(LOG_FATAL, _("Tech %s does not exist, but is tech for %s."),
+get_tech_name(NULL, tech), pnation->name);
+exit(EXIT_FAILURE);
+ }
+ if (advances[tech].root_req != A_NONE
+ && !nation_has_initial_tech(pnation, advances[tech].root_req)) {
+/* Nation has no root_req for tech */
+freelog(LOG_FATAL, _("Tech %s is initial for %s, but they have "
+ "no root_req for it."),
+get_tech_name(NULL, tech),
+pnation->name);
+exit(EXIT_FAILURE);
+ }
+}
+ } players_iterate_end;
+
+ return TRUE;
+}
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#38323) Crash when initial techs contain unreachable tech
http://bugs.freeciv.org/Ticket/Display.html?id=38323 >
civserver: ../../src.patched/server/techtools.c:278: found_new_tech:
Assertion `tech_is_available(plr, tech_found) || tech_found ==
(200-2)' failed.
#3 0x00415b1f in found_new_tech (plr=0x760c48, tech_found=5,
was_discovery=false, saving_bulbs=true)
at ../../src.patched/server/techtools.c:278
next_tech =
bonus_tech_hack =
was_first =
had_embassies = {0, 0, 4897441, 0, 84, 0, 4897441, 0, 1, 0,
4898788, 0, 2034368, 1077692614, 4897441, 0, 1, 0, 4898788, 0, 2034368,
1077692614, 6976164, 0, 41, 0, 7819944, 0, 0, 0, 7819948, 0}
pcity =
research = (struct player_research *) 0x6a9234
__PRETTY_FUNCTION__ = "found_new_tech"
#4 0x00415c82 in give_initial_techs (plr=0x760c48)
at ../../src.patched/server/techtools.c:614
nation = (struct nation_type *) 0x8b7600
#5 0x0040c0da in srv_main ()
at ../../src.patched/server/srv_main.c:1889
No locals.
#6 0x00403868 in main (argc=3, argv=0x7fff9cd344a8)
at ../../src.patched/server/civserver.c:256
inx = 3
showhelp = false
showvers = false
option =
Nation does not have root_req for its initial tech. At least ruleset
sanity checking is needed so that game gives sensible error message to
ruleset author. This is not trivial as we need to check if *nations*
are legal, but tech_is_available() works on *players*.
It also seems to me that this crash might happen if initial root_req
is simply processed later than than tech depending on it.
Found while testing Mongols scenario...
- ML
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
