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

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

At 05:28 AM 02/01/04 -0500, Jason Short wrote:
>Ross W. Wetmore wrote:
>
>> At 07:51 AM 02/01/03 -0500, Jason Short wrote:
>>>Ross W. Wetmore wrote:
>>>>At 03:44 AM 02/01/02 -0500, Jason Short wrote:
>
>[a lot of excerpts...]
>
>>>if 
>>>non-normal positions are ever passed in eventually (-1,-1) will be 
>>>legitimately passed in and will be treated as a nonexistent position. 
>>>
>> 
>> Non-normal is meaningless. I repeat, non-normal is meaningless, I repeat
>> non-normal is meaningless ... get it through your skull :-)
>
>"Normal" only has meaning because we choose to use it; it has no 
>inherent value. 

If you take this further, then any time you replace an is_real_tile()
check which does have a hard meaning, with some arbitrary semantic
meaning chosen for no inherent value, then something has been lost.

>If the position (-1,-1) is allowed to be passed in as a legitimate 
>position, it will be impossible for the code (either server code in 
>vnotify_conn_ex or the client code that receives the packet) to 
>determine whether this (-1,-1) is a legitimate real position or a 
>special-case to mark the even as having no coordinates associated with 
>it.  This is a bug, and a very hard one to track down.

This "bug" is not present in this code. It cannot be fixed here. In
any event most of the cases like ENOEVENT do not have any bug. The 
context is quite clear and handled properly by current code.

>OTOH, if we say that the legitimate positions passed in must be "normal" 
>and specify that (-1,-1) can never be "normal", then we don't have this 
>problem.  However, code may pass in legitimate "non-normal" positions, 
>causing the code to fail...but since we'll get a failed assertion in 
>this case the bug is very easy to track down.

Your fix breaks the correct codepaths above. This is not the place to
start finding any other bugs, and not the correct fix to do this.

Just fix the local problem here, please. Don't introduce new ones.


>jason

Cheers,
RossW
=====