Re: [Freeciv-Dev] (PR#39556) assert padvance failed

2007-08-13 Thread William Allen Simpson

http://bugs.freeciv.org/Ticket/Display.html?id=39556 >

Marko Lindqvist wrote:
>  Isn't that obvious from the caller?
> 
>  It's the tech making wonder obsole, probably none. Is
> buildings.ruleset loading code up to your iterator changes?
> 
Too many commits since your line numbers, I was looking at the wrong line.

Yes, that test should be done as pointers.  Fixed.

Moved the test to be first (before checking for wonder), as that quick
test obviates the need for all of the rest.

Also, the test for A_FUTURE should be *before* checking state.  It says so
in the comments, but was backward in the assert!

Also, checked for every instance of obsolete_by, found another case that
wasn't checked for validity before use -- in gui-mui, and the line before
it won't compile, either.  So, I just flagged it with FIXME.

Then, after committing those fixes, it seemed to me that we shouldn't
bother running the loop for A_FUTURE (or other invalid) advances.  So,
a quick one line commit to fix it.

Ha, identified four bugs with one assert!  Keep up the good work.

Index: server/techtools.c
===
--- server/techtools.c  (revision 13297)
+++ server/techtools.c  (working copy)
@@ -270,13 +270,13 @@
   bool can_switch[MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS]
  [government_count()];
   struct player_research *research = get_player_research(plr);
+  struct advance *vap = valid_advance_by_number(tech_found);
 
   /* HACK: A_FUTURE doesn't "exist" and is thus not "available".  This may
* or may not be the correct thing to do.  For these sanity checks we
* just special-case it. */
-  assert((valid_advance_by_number(tech_found)
- && player_invention_state(plr, tech_found) != TECH_KNOWN)
-|| tech_found == A_FUTURE);
+  assert(tech_found == A_FUTURE
+|| (vap && player_invention_state(plr, tech_found) != TECH_KNOWN));
 
   /* got_tech allows us to change research without applying techpenalty
* (without loosing bulbs) */
@@ -290,9 +290,9 @@
   if (was_first) {
 /* Alert the owners of any wonders that have been made obsolete */
 improvement_iterate(pimprove) {
-  if (is_great_wonder(pimprove)
+  if (vap == pimprove->obsolete_by
+ && is_great_wonder(pimprove)
  && great_wonder_was_built(pimprove)
- && advance_number(pimprove->obsolete_by) == tech_found
  && (pcity = find_city_from_great_wonder(pimprove))) {
notify_player(city_owner(pcity), NULL, E_WONDER_OBSOLETE,
 _("Discovery of %s OBSOLETES %s in %s!"), 
Index: client/gui-mui/helpdlg.c
===
--- client/gui-mui/helpdlg.c(revision 13297)
+++ client/gui-mui/helpdlg.c(working copy)
@@ -642,8 +642,8 @@
 DoMethod(help_wonder_cost_text, MUIM_SetAsString,
 MUIA_Text_Contents, "%ld", impr_build_shield_cost(imp));
 
-UpdateTechButton(help_wonder_needs_button, advance_number(imp->tech_req));
-UpdateTechButton(help_wonder_obsolete_button, 
advance_number(imp->obsolete_by));
+UpdateTechButton(help_wonder_needs_button, FIXME 
advance_number(imp->tech_req));
+UpdateTechButton(help_wonder_obsolete_button, FIXME 
advance_number(imp->obsolete_by));
   }
   else
   {
Index: server/techtools.c
===
--- server/techtools.c  (revision 13299)
+++ server/techtools.c  (working copy)
@@ -287,7 +287,7 @@
   research->techs_researched++;
   was_first = (!game.info.global_advances[tech_found]);
 
-  if (was_first) {
+  if (was_first && vap) {
 /* Alert the owners of any wonders that have been made obsolete */
 improvement_iterate(pimprove) {
   if (vap == pimprove->obsolete_by
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#39556) assert padvance failed

2007-08-13 Thread Marko Lindqvist

http://bugs.freeciv.org/Ticket/Display.html?id=39556 >

On 13/08/07, William Allen Simpson <[EMAIL PROTECTED]> wrote:
>
> Also, checked for every instance of obsolete_by, found another case that
> wasn't checked for validity before use -- in gui-mui, and the line before
> it won't compile, either.  So, I just flagged it with FIXME.

 You do know that gui-mui has been out of use for a extremely long
time? It doesn't hurt to make simple search/replace corrections there
too, but for otherwise your time is probably better spent developing
other parts of the code.
 From the commit logs I'd guess that last time gui-mui was in
compiling state was 7 years ago.


 - ML



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#39556) assert padvance failed

2007-08-13 Thread Marko Lindqvist

http://bugs.freeciv.org/Ticket/Display.html?id=39556 >

On 13/08/07, William Allen Simpson <[EMAIL PROTECTED]> wrote:
>
> Marko Lindqvist wrote:
> >  I have some patches applied (meaning line numbers may not match svn
> > version), but they are unlikely cause.
> >
> Aha, the new asserts are working!
>
> Now, somebody has to figure out why a NULL is in the variable.  I don't
> think it should be there

 Isn't that obvious from the caller?

 It's the tech making wonder obsole, probably none. Is
buildings.ruleset loading code up to your iterator changes?


 - ML



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#39556) assert padvance failed

2007-08-13 Thread William Allen Simpson

http://bugs.freeciv.org/Ticket/Display.html?id=39556 >

Marko Lindqvist wrote:
>  I have some patches applied (meaning line numbers may not match svn
> version), but they are unlikely cause.
> 
Aha, the new asserts are working!

Now, somebody has to figure out why a NULL is in the variable.  I don't
think it should be there



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#39556) assert padvance failed

2007-08-12 Thread Marko Lindqvist

http://bugs.freeciv.org/Ticket/Display.html?id=39556 >

 I have some patches applied (meaning line numbers may not match svn
version), but they are unlikely cause.


#0  0x2b15100b8635 in raise () from /lib/libc.so.6
No symbol table info available.
#1  0x2b15100ba090 in abort () from /lib/libc.so.6
No symbol table info available.
#2  0x2b15100b1c2f in __assert_fail () from /lib/libc.so.6
No symbol table info available.
#3  0x004126a9 in advance_number (padvance=)
at ../../src.test/common/tech.c:91
__PRETTY_FUNCTION__ = "advance_number"
#4  0x004714c0 in found_new_tech (plr=0x876eb0, tech_found=34,
was_discovery=true, saving_bulbs=true)
at ../../src.test/server/techtools.c:293
id = 60
bonus_tech_hack = 
was_first = true
had_embassies = {9023568, 0, 5385397, 0, 12112800, 0, 0, 0, 27, 0,
  4268721, 0, 1, 0, 4269118, 0, 8875696, 0, 0, 1082392576, 643, 0,
  8875696, 0, 8988032, 0, 7848520, 0, 8875696, 0, 4660692, 0}
pcity = (struct city *) 0xa750e0
research = (struct player_research *) 0x77c248
__PRETTY_FUNCTION__ = "found_new_tech"
#5  0x004720f8 in update_tech (plr=0x876eb0, bulbs=0)
at ../../src.test/server/techtools.c:460
research = (struct player_research *) 0x77c248
#6  0x0048320d in update_city_activities (pplayer=0x876eb0)
at ../../src.test/server/cityturn.c:1570
g = (struct government *) 0x924430
myiter = (struct genlist_link *) 0xa57240
pcity = 
gold = 256
#7  0x004677a7 in main_loop ()
at ../../src.test/server/srv_main.c:743
pplayer = (struct player *) 0x876eb0
eot_timer = (struct timer *) 0xc410f0
save_counter = 86
is_new_turn = 
__PRETTY_FUNCTION__ = "main_loop"
#8  0x00467fc4 in srv_main ()
at ../../src.test/server/srv_main.c:2062
No locals.
#9  0x00403a99 in main (argc=3, argv=0x7fff9b527bf8)
at ../../src.test/server/civserver.c:275
inx = 3
showhelp = false
showvers = false
option = 


 - ML



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev