[Freeciv-Dev] Re: Freeciv Networking/Mac Server patches

[David Pfitzner <dwp@xxxxxxxxxxxxxx>]

Andy Black <ablack@xxxxxxxxxxxxxx> wrote:
> 
> The attached file is a group of diffs that I used to make the server
> compile on the mac.

I commited some of the more simple/straightforward changes.

Some comments on some of the rest:

> civserver.c.diff
>       A version of the mac option dialog (#ifdefed out due to crashes,
> hardcoded version used insted, will work on later)

This should be easier to do differently soon, when srv_main.c is
added and there will be a 'struct server_arguments' which you can
adjust/place parameters in.

>       added gethostname check

I would prefer that you add a function mygethostname() in support.c,
which returns 0 if not HAVE_GETHOSTNAME.

        sz_strlcpy(metaserver_servername, option);
+         /*why sz_strlcpy and not = ? Is it the difference betwene
+         having a char [] and a char* for the dest?*/

In a word: yes.  :-)  Ie, that is the difference.

> config.mac.h.diff
>       updated version.  added recent items from config.h.in, other mac
> only changes

Ok.  (I don't really care what you do in this file :-)

> console.h.diff
>       updated protos for move

 /* make sure a prompt is printed, and re-printed after every message */
-void con_prompt_on(void);
+/*void con_prompt_on(void);*/
+       /*not needed outside console.c*/

In such cases you can just remove the prototype from the header file
and make the function 'static' in the .c file (with a static prototype 
inside the .c file if necessary).

> diplhand.c.diff
>       small (pointless) cleanup of handle_diplomacy_accept_treaty.
>       removed use of extranious data.  remove sending of data after next
> stable version

I agree the check is unnecessary, but its also harmless.  Plus if we
actually did something with the check (eg, log message) this could be 
useful to notice a buggy client, if the client accidently fills in a 
wrong player_from.  Removing sending of the data is probably not worth 
doing, because the diplomacy things all send the same packet type, so 
this would require adding a new packet etc.  Unless one did a more 
extensive rework.

More worthwhile would be increased checking of the other diplmoacy
parameters: it seems to me that a third party could insert a clause
into the treaty of two other players, as the server doesn't check
that pplayer is either plr0 or plr1?  (And since it should be, then 
one of plr0 or plr1 is always redundant?)

Incidently I would have thought a two-stage acceptance would be 
safer: what if someone adds a clause just _before_ the server 
receives your 'accept' packet (and then they quickly accept 
themselves). 

> mem.c.diff
>       a few mac (client) related items.
> mem.h.diff
>       proto for failure_cleanup

Since failure_cleanup() doesn't currently do anything and is not
called from anywhere else, I'm not that keen on adding it...

> netintf.h.diff
>       added enums and typedefs for generalized networking
>       added protos for generalized networking

What is TEP supposed to stand for?

-- David