Complete.Org: Mailing Lists: Archives: freeciv-dev: March 2003:
[Freeciv-Dev] Re: client/server authentication (PR#1767)
Home

[Freeciv-Dev] Re: client/server authentication (PR#1767)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: kaufman@xxxxxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: client/server authentication (PR#1767)
From: "Per I. Mathisen" <per@xxxxxxxxxxx>
Date: Sat, 22 Mar 2003 06:44:26 -0800
Reply-to: rt@xxxxxxxxxxxxxx

On Sat, 22 Mar 2003, Mike Kaufman wrote:
> auth0-1b.diff: moves the connection parts of the server out of plrhand,c
>              and srv_main.c into their own file connecthand.c. This is
>              really nice. The patch is fairly simple to comprehend.

Please fix style issues in code you move around.

> auth1-2a.diff: does some name changing to make things make more sense.
>              a request to join a game becomes a request to login. That's
>              pretty much it.

Looks perfectly ok.

> auth2-3c.diff: more name changing. a new command. merge some functions in
>              connecthand.c.

A very brief look only, but looks ok. A few style nitpicks, though. Code
like this

+  if (strlen(req->name) == 0 || my_isdigit(req->name[0]) ||
+      strcasecmp(req->name, "all") == 0 ||
+      strcasecmp(req->name, "none") == 0 ||
+      strcasecmp(req->name, "nouser") == 0) {

Should be

+  if (strlen(req->name) == 0
+      || my_isdigit(req->name[0])
+      || strcasecmp(req->name, "all") == 0
+      || strcasecmp(req->name, "none") == 0
+      || strcasecmp(req->name, "nouser") == 0) {

Also, please capitalize comments properly (I know I sin in this respect as
well).

I hope to give more constructive comments later on.

  - Per




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