bugFreeciv - Bugs: bug #21418, Ruleset loading requirement...

 
 
Show feedback again

bug #21418: Ruleset loading requirement validity check relies on information not yet loaded

Submitted by:  Jacob Nevins <jtn>
Submitted on:  Fri 03 Jan 2014 12:44:45 AM UTC  
 
Category: NoneSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: Jacob Nevins <jtn>Open/Closed: Closed
Release: Operating System: Any
Planned Release: 2.4.2,2.5.0,2.6.0

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Sun 05 Jan 2014 10:59:02 AM UTC, SVN revision 24046:

Defer checking of improvement requirement ranges until all ruleset
information has been loaded, because it can't be done reliably before
that point.

See gna bug #21418.

(Browse SVN revision 24046)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Sun 05 Jan 2014 10:54:48 AM UTC, SVN revision 24034:

Defer checking of improvement requirement ranges until all ruleset
information has been loaded, because it can't be done reliably before
that point.

See gna bug #21418.

(Browse SVN revision 24034)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Sun 05 Jan 2014 10:47:11 AM UTC, SVN revision 24020:

Defer checking of improvement requirement ranges until all ruleset
information has been loaded, because it can't be done reliably before
that point.

See gna bug #21418.

(Browse SVN revision 24020)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Fri 03 Jan 2014 05:15:08 AM UTC, comment #2:

Now we have working ruleset load time checks, I've removed i18n marking from some runtime error messages about failure to meet these conditions as (a) they should never happen (b) we already have a policy that says log_error() usually isn't initialised (c) they're very obscure and hard to translate.

(file #19624)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Fri 03 Jan 2014 01:42:00 AM UTC, comment #1:

> Probably this check needs to be deferred to
> sanity_check_ruleset_data(), once all the data is loaded.


On a general note, we need to be moving all possible sanity checks to sanity_check_ruleset_data() which freeciv-ruledit then can use to check ruleset being modified.

Marko Lindqvist <cazfi>
Project Administrator
Fri 03 Jan 2014 12:44:45 AM UTC, original submission:

In req_from_str(), some validity checking is done on the requirement, particularly whether its range is appropriate for its type.

In the case of VUT_IMPROVEMENT, the valid ranges depend on whether it's a wonder or not. Unfortunately, the information checked here is not guaranteed to have been loaded yet; req_from_str() is called during ruleset loading, and depending on context, some or all improvements may not have been read yet.

It's been like that since this code was added ([http://svn.gna.org/viewcvs/freeciv?revision=10269&view=revision r10269, PR#12823, 2005-04).

In my testing, the checks seemed to pass incorrectly, probably because IG_GREAT_WONDER == 0 and the improvement data structures are statically allocated and hence zeroed. However, I suspect that if a ruleset is loaded a second time, this check could spuriously fail a valid ruleset.

Probably this check needs to be deferred to sanity_check_ruleset_data(), once all the data is loaded.

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #19640:  S2_5-rssanity-improvement-req-range.patch added by jtn (11kB - text/x-diff - trunk/S2_5/S2_4 r24003)
file #19641:  S2_4-rssanity-improvement-req-range.patch added by jtn (11kB - text/x-diff - trunk/S2_5/S2_4 r24003)
file #19624:  trunk-rssanity-improvement-req-range.patch added by jtn (11kB - text/x-diff - trunk r23987 + bug #21419)

 

Digest:
   bug dependencies.

Digest:
   patch dependencies.

 

Carbon-Copy List
  • -unavailable- added by cazfi (Posted a comment)
  • -unavailable- added by jtn (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 05 Jan 2014 11:03:21 AM UTCjtnStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Fri 03 Jan 2014 08:06:30 PM UTCjtnAttached File-=>Added S2_5-rssanity-improvement-req-range.patch, #19640
      Attached File-=>Added S2_4-rssanity-improvement-req-range.patch, #19641
      StatusIn Progress=>Ready For Test
      Planned Release=>2.4.2,2.5.0,2.6.0
    Fri 03 Jan 2014 05:15:21 AM UTCjtnDependencies-=>Depends on bugs #21419
    Fri 03 Jan 2014 05:15:08 AM UTCjtnAttached File-=>Added trunk-rssanity-improvement-req-range.patch, #19624
      Operating SystemNone=>Any
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup