Complete.Org: Mailing Lists: Archives: freeciv-dev: March 2003:
[Freeciv-Dev] Re: (PR#3649) Network usage optimization
Home

[Freeciv-Dev] Re: (PR#3649) Network usage optimization

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: arnstein.lindgard@xxxxxxx
Subject: [Freeciv-Dev] Re: (PR#3649) Network usage optimization
From: "Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>
Date: Fri, 7 Mar 2003 20:18:50 -0800
Reply-to: rt@xxxxxxxxxxxxxx

Arnstein Lindgard wrote:
> On Fri, 7 Mar 2003 10:43:08 -0800
> "Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx> wrote:
> 
> 
>>- I'd like to see assertions somewhere to verify that the variables do 
>>fall within the smaller bounds.  Perhaps this is going too far, though; 
>>it seems obvious that they do.
> 
> 
> At first glance it seems obvious. I can demonstrate that none of
> the int's are likely to exceed 255, which is uint8 limit.

I agree, it's pretty obvious that they work.

> Or are you just saying it would be good coding practice generally to
> use asserts when you change something fundamental, and leave it in
> there for awhile? I'm not sure why and where to put any asserts here.

My point is that this constraint is not obvious from a global point of 
view.  Say someone were going to change the enum diplstate_type into a 
flag structure, or allow "unlimited" turns for turns_left - they 
probably wouldn't think of the network limitation and would end up with 
difficult-to-trace bugs.  These can be easily avoided by adding an 
assertion.

But, I think this can be addressed at a lower level.  The dio_put_int 
functions all take int* values, so it should be easy to add an assertion 
within them to verify the range (which doesn't need to be a part of this 
patch).  That's what I think we should do (if it isn't done already...). 
  Since this code is called often we may wish to only enable it if DEBUG 
is defined.

jason




[Prev in Thread] Current Thread [Next in Thread]