[Freeciv-Dev] Re: [PATCH] bug in vnotify_conn_ex() (PR#1185)

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

At 10:40 AM 02/01/06 +0100, Raimar Falke wrote:
>On Sat, Jan 05, 2002 at 11:47:45PM -0500, Ross W. Wetmore wrote:
>> At 08:00 PM 02/01/05 -0500, Jason Short wrote:
>> >Ross W. Wetmore wrote:

>> There is no reason not to include (-1,-1) in a normal set because as you 
>> keeping saying, it is arbitrary how you define "normal", unlike real
>> which does have a hard meaning.
>
>I think that we agreed that for normal position (x,y) "0<=x<map.xsize
>and 0<=y<map.ysize" is always true. If we also assume that the
>function vnotify_... takes normal position OR this special case it is
>possible to distinguish between the special case, normal positions and
>misuses of the function (any other case).

This is certainly not always true nor previously agreed. 

It is also an unnecessary restriction made feasible only because you have 
decided that robustness should be sacrificed for raw speed.

The decision to optimize all coordinates access by normalizing at source and
not checking downstream now means you are dying all over the place because
you didn't properly analyze and take care of all the critical conditions up 
front, or limit yourself to the places where this was reasonable.

You were also warned that this was pushing things further than you were
ready to deal with.

The optimization is fine in places where it is checked out and made safe.
That is not the issue.

It is also equally fine to deal with unnormalized coordinates and normalize 
them only when needed, rather than always. This has been done in the past
and works quite well. To say different is a religious and not a technical 
position.

Note ...

The test for normalized is currently faster than your current checks. Also
since the value will only be normalized once at any point in its entire 
codepath there is not a lot of performance difference between the up front 
expense and the lazy normalization technique using a properly coded 
normalize_map_pos().

It *is* better to do a normalize_map_pos() up front to deal with the unreal
case, and as a bonus get the normalization as well. But it is also not
unreasonable to just do the realness check and defer normalization. In
some cases you actually want to *not* normalize.

It won't hurt to have a few extra robustness checks at various key points 
either. The *input* functions in the client that receive values should 
certainly be a place for this. Protecting with a server fix under the 
mistaken assumption that clients then don't need to worry about robustness
is a foolish mistake most people learn to avoid with maturity.

>       Raimar
>-- 
> email: rf13@xxxxxxxxxxxxxxxxx
> "The BeOS takes the best features from the major operating systems. 
>  It's got the power and flexibility of Unix, the interface and ease 
>  of use of the MacOS, and Minesweeper from Windows."

Cheers,
RossW
=====