Complete.Org: Mailing Lists: Archives: freeciv-dev: January 2004:
[Freeciv-Dev] Re: Extended connect dialog (PR#977)
Home

[Freeciv-Dev] Re: Extended connect dialog (PR#977)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: dspeyer@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: Extended connect dialog (PR#977)
From: "Mike Kaufman" <kaufman@xxxxxxxxxxxxxxxxxxxxxx>
Date: Mon, 19 Jan 2004 07:46:06 -0800
Reply-to: rt@xxxxxxxxxxx

<URL: http://rt.freeciv.org/Ticket/Display.html?id=977 >

On Mon, Jan 19, 2004 at 01:10:44AM -0800, Jason Short wrote:
> 
> - Your translation code is wrong.
> 
ah, so someone finally explains...

> - Why do the packets you introduce require explicit (non-delta) 
> handling?  Raimar?

the handling is explicit, but they are delta packets. the reason for these
flags: handle-per-conn,no-handle is that these packets are sent before
players are assigned. They must be handled on a connection rather than
player basis. They must also be handled before the !pplayer check is
performed. I've talked to Raimar about added another packet handler for
these type of packets. He agrees.

> - /* FIXME: may not be long enough? use MAX_PATH? */
> 
> You should use PATH_MAX or (better) dynamically assign arrays that store 
> a path.  Note that PATH_MAX doesn't exist on all systems; if it doesn't 
> exist then the path length is unbounded.  (I'm pretty sure it's 
> PATH_MAX, defined in limits.h, not MAX_PATH.)

True enough. I must have gotten discombobulated, cause I wanted to use the
OS define... I don't think that dynamically assigned is going to work here
cause you have to use the array in getopt.

Q: Raimar, do we have array maxes in delta?
   Will the dataio put_string function automatically do a strlen?

> - This patch introduces a lot of design that needs documentation, 
> probably in doc/HACKING.  Better code comments will help too: not just 
> saying what a function does but how it fits into the overall design.

Ok.




[Prev in Thread] Current Thread [Next in Thread]