bugFreeciv - Bugs: bug #24081, Ruleset loading could validate...

 
 
Show feedback again

bug #24081: Ruleset loading could validate against network protocol limits

Submitted by:  Jacob Nevins <jtn>
Submitted on:  Sun Nov 15 12:31:13 2015  
 
Category: NoneSeverity: 3 - Normal
Priority: 5 - NormalStatus: None
Assigned to: NoneOpen/Closed: Open
Release: Operating System: Any
Planned Release: Contains string changes: None

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

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

 

Sun Nov 15 13:51:31 2015, comment #3:

Aha, so the cheap thing to do is also the right thing :)

Jacob Nevins <jtn>
Project Administrator
Sun Nov 15 12:43:40 2015, comment #2:

> can only be done once all the ruleset fields for a packet have been loaded


Which would fit the general preference that validation takes place in rssanity.c for the benefit of getting the check apply to ruledit too, wouldn't it?

Marko Lindqvist <cazfi>
Project Administrator
Sun Nov 15 12:41:59 2015, comment #1:

Also, ruledit should probably do the same thing.
(Which means the struct->packet field name mapping goes in an extra place.)

Jacob Nevins <jtn>
Project Administrator
Sun Nov 15 12:31:13 2015, original submission:

In bug #24074, a ruleset specified a unit move_rate that was legal on the server but didn't fit in the network protocol.

In general, we don't catch this currently. The network protocol representation is often the bottleneck. It might be a good idea if ruleset loading checked against network limits in easy/common cases.

To avoid duplicating type information in packets.def and ruleset loading code, I think this requires generate_packets.py to output some representation of the underlying type limitations that can be used in macros, so that we can change code like

to

which complains if the number doesn't fit in a UINT8 (PACKET_RULESET_UNIT.defense_strength): and more complicated cases looking like

+verbatim-

|| !secfile_lookup_int(file, &u->move_rate,
"%s.move_rate", sec_name)

/* ... */

u->move_rate *= SINGLE_MOVE;
NETVALIDATE_INT(u->move_rate, RULESET_UNIT, move_rate);
-verbatim-

A cheaper way of achieving the same end would be to do a trial run through the packet serialisation code when loading rulesets, but that would have less good diagnostics (unless we do bug #24080 too), and can only be done once all the ruleset fields for a packet have been loaded and munged.

Jacob Nevins <jtn>
Project Administrator

 

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

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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.

     

    Error: not logged in

     

     

    No Changes Have Been Made to This Item
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup