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

["Per I. Mathisen" <Per.Inge.Mathisen@xxxxxxxxxxx>]

On Wed, 2 Jan 2002, Jason Short wrote:
> 1.  Calling is_real_tile before the game starts is generally no good.
> Under general-topologies, it will result in a failed assertion as the
> topology will not yet have been initialized.  Thus it should be clear
> that the order here should be reversed.
>
> 2.  Again, is_real_tile may return true for (-1,-1) under an alternate
> topology.  The easy solution here is just to change it to
> is_normal_map_pos.  There are still some minor pitfalls: (1) it
> obviously won't work if is_normal_map_pos(-1,-1), and (2) we need to be
> careful about non-normal positions other than (-1,-1) being passed in;
> these would probably indicate a bug.  Both of these can be detected with
> a well-placed assertion.
>
> The attached patch fixes both of these issues.
...
>    conn_list_iterate(*dest, pconn) {
> -    if (is_real_tile(x, y) && server_state >= RUN_GAME_STATE
> +    if (server_state >= RUN_GAME_STATE && is_normal_map_pos(x, y)
>         && ((pconn->player==NULL && pconn->observer)
>             || (pconn->player!=NULL && map_get_known(x, y,
> pconn->player)))) {
> +      /* If is_normal_map_pos(-1,-1), this assertion will catch the bug.
> */
> +      assert(x != -1 || y != -1);
>        genmsg.x = x;
>        genmsg.y = y;
>      } else {
> +      /* Any invalid coordinates other than (-1, -1) are a bug. */
> +      assert((x == -1 && y == -1) || is_normal_map_pos(x, y));
>        genmsg.x = -1;
>        genmsg.y = -1;
>      }

If this is an indication of a bug elsewhere in the code, should this
result in an assert() failure, or at least be logged as a warning?

Yours,
Per

Action expresses priorities.
 -- Mohandas Gandhi