[Freeciv-Dev] Re: [Patch] Stricter input checking (PR#1764)

[Jason Short <vze2zq63@xxxxxxxxxxx>]

Ross W. Wetmore wrote:
> This is a foolish mistake.
>
> In general, requiring realness in virtually all cases is not bad.
> Requiring normalized in any but carefully orchestrated ones is very
> bad.
>
> Since normalize_map_pos() not only does a fast check for real,

Theoretically, is_normal_map_pos is faster than normalize_map_pos.

> but also insures the output coordinates are normalized, there is
> absolutely no need for an is_normal_map_pos test anywhere in the
> code - ony realness tests using normalized_map_pos() are needed.
> The replacements here are in fact errors waiting to happen and
> will bring back the chaos of the void tile years where realness
> was generally ignored.
>
> Is_normal_map_pos was only added for debug purposes and was recognized
> at the time as having no valid use outside of this.

Define "valid".

> It is an
> incomplete test for validity. It's use here is thus still invalid.
>
> Realness of coordinates is a hard condition that can only be dealt
> with at source and is an error elsewhere. Normalized is a soft and
> easily remedied one that can be handled anywhere in code - for example
> as local efficiency mods. The first *is* required of game coordinates.
> The second is meaningless by itself as an incomplete test.

This is true but not particularly meaningful.  As long as "normal" has a
consistent meaning within the code, assuming coordinates have been
normalized and just doing the is_normal check can save time and
(occasionally) code.

> Normalized
> is also not a rigorously defined concept while real is. Thus "passing
> normalized coordinates" is only useful wrt a local definition of
> normalized.

This is a very valid point, and a decent reason not to use 
is_normal_map_pos in this case.  Since this code interfaces the client 
with the server, the use of is_normal here assumes that the client and 
server have the same definitions of "normal" - which is probably a safe 
assumption, but an unnecessary one.
jason