[Freeciv-Dev] Re: PATCH: remove check_coords (PR#1013)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Sat, Oct 13, 2001 at 10:27:07PM -0700, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
> This patch removes the check_coords function, in preparation for it to
> be replaced by a better check_map_pos macro/function.
> 
> None of the current places that use check_coords is using it correctly,
> anyway.
> 
> Note that in one place I have done
> 
> -  /* It is ok placing check_coords() inside an assert (ie may be
> optimized
> -     away by compiler). Though it may adjust dest_x, dest_y it
> shouldn't
> -     be needed. */
> -  assert(check_coords(&dest_x, &dest_y));
> +  assert(is_normal_map_pos(dest_x, dest_y));
> 
> Although the comment claims that the earlier code is correct, the
> comment is wrong and the earlier code is a bug.  check_coords returns
> TRUE any time the passed coordinates are real, regardless of whether
> they had to be normalized.  Thus the original code was nearly the
> equivalent of assert(normalize_map_pos(&dest_x, &dest_y)).  It's
> probably true that this doesn't matter since the coordinates are always
> normal.
> 
> Raimar: we had discussed writing the check_map_pos macro before removing
> check_coords, but since the check_coords removal is more straightforward
> I made this patch before.  If you still want to do the other change
> first, this one can wait.  Note that only in two situations (the one
> above and possibly one other) should check_map_pos be substituted in for
> check_coords, so there's not as much conflict between these two changes
> as you might expect.

The order in which the patches go in doesn't matter.

        Raimar

> -  if (!check_coords(&req->x, &req->y))
> -    return;
> +  if (!normalize_map_pos(&req->x, &req->y))
> +    return; /* discard invalid packet */

We want to be more strict. We can expect that good clients will only
sent normal(ized) map positions. However there is a problem (can't
there be something without a problem?): old client may still sent map
positions that are real but not normal(ized). Maybe we can make the
above checks with is_normal_map_pos and than try the 1.12.0 client?!

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  Windows: From the people who brought you edlin...