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

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

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:
> >
> >> You still don't understand that ENOEVENT (-1,-1) is a perfectly valid
> >> case, whether or not your topology allows (-1,-1) as a real tile. It 
> >> is required that this be passed on. There are other cases as well. The
> >> value is used as a signal, independent of topology hardwired into lots 
> >> of code.
> >> 
> >> Thus you don't fix the original bug, but just kill the server at a
> >> different point. The original problem was ENOEVENT (-1,-1) in a torus
> >> world if you read the comments. They also explain ENOEVENT syntax.
> >> 
> >> (-1,-1) *is* a special case that *must* just be passed through. All
> >> the asserts that specifically target this are dead wrong. This "fix"
> >> is not only not required but breaks valid behaviour. In fact (-1,-1) 
> >> is the only unnormalized coordinate you absolutely *must* not touch.
> >> 
> >> Both Raimar and your zeal to stamp out unnormalized coordinates is 
> >> exemplary, but misplaced in this case. Leave this code out of your 
> >> targets! 
> >
> >I think you are misreading the code.  (-1,-1) _is_ treated as a 
> >special-case for a nonexistent tile.  It fails the check and so is 
> >passed through as (-1,-1) directly to the client.
> 
> Sorry, but in a torus world your assert kills the server. Remove the
> assert for the (-1,-1) case and just pass it through untouched, unchecked
> and however unloved. It does not need to be 1984ed or purified by the
> normalizing assert benediction.
> 
> You have still changed the behaviour with the assert for non-special
> cases which were once allowed and now are not. There is no reason to
> do this here other than misplaced zeal. There is *never* any reason
> to force normalization other than at the original generation point
> or before you access an interface that requires normalized values.
> None exist here, ergo it is not needed.
> 
> Note, non-normal coordinates are not a problem because they can 
> *always* be fixed at any point in the code in a completely standard
> fashion - this is the way everything used to work and it still works
> if you need it.
> 
> Unreal coordinates *are* a problem because only the generator of the
> unreal position has any reasonable chance of dealing with the failure
> codepath. Hence is_normal_map_pos() is usually a mistake because it
> misses fixable problems, and cannot detect the unfixable ones.
> 
> >The problem is that if (-1,-1) is passed in as a legitimate set of 
> >coordinates, it will be treated as the special-case of no coordinates 
> >associated with the event.  Thus it is very important that we stamp out 
> >non-normal coordinates being passed to this function!  Of course, we 
> >must allow (-1,-1) to be passed but if any _other_ non-normal 
> >coordinates are passed in then we're in danger.  This is a bug in the 
> >calling code that can only be caught here.
> >
> >jason
> 
> >+  /*
> >+   * Hack: we special-case the coordinates (-1,-1) to mean the event has
> >+   * no coordinates associated with it.  This works just fine, so long
> >+   * as (-1,-1) isn't a "real" map position. 
>                           ^^^^^^
> In a torus world, -1,-1 is a real position. ENOEVENT(-1,-1) is a perfectly
> valid event.
> 

> 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).

        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."