On Wed, 2010-01-06 at 22:02 +0100, Bas Wijnen wrote: > On Fri, Nov 13, 2009 at 09:24:04PM +0100, Roland Clobus wrote: > > I see a few comments about modifying the patch already sent. What will > > the actual patch be for me to test? > > I have it attached. I wanted to make it clean by doing make reindent > first, but somehow it turned it into a 900+ lines diff. Sorry about > that. The patch is identical to my earlier patch plus the suggested > addition for IPV6_V6ONLY in common/network.c.
I've noticed that 'make reindent' creates big patch files too, it is fixed in svn revision 1495 (by invoking indent twice). Attached you'll find the simplified version. > > > (In the meantime I've released 0.12.3, but a 0.12.4 could follow soon, > > if required) > > The patch does add a new string (which shouldn't be seen, but should be > translated), so that will need a string-freeze waiting period. String freezes will be no problem, there are already modified strings for 0.12.4. A release in Debian of 0.12.3 would be nice (ergo not waiting for 0.12.4), because many users would then be able to play games with a server on localhost again. Now about the requested functionality for IPv6 support. It is not clear to me what the required behaviour would need to be (regarding Debian). What should the server do when either the IPv4 or IPv6 socket is already in use? I would recommend that the server will refuse to be run at all, otherwise situations would exist where the client would connect with the wrong protocol to another listening process, see the scheme below. I've tried to chart the behaviour. I've started pioneers-server-gtk on port 5550 in combination with 'nc -4 -l localhost 5550' and/or 'nc -6 -l localhost 5550'. In the scheme: 4O6F means: IPv4 occupied by netcat, IPv6 free For the unpatched server: 4F6F: listen on IPv4, client connects on IPv4 4O6F: listen on IPv6, client connects on IPv6 4F6O: listen on IPv4, client connects on IPv6 -> situation 1 4O6O: situation 2 For the patched server: 4F6F: listen on IPv4 and IPv6, client connects on IPv6 4O6F: listen on IPv6, client connects on IPv6 -> situation 3 4F6O: listen on IPv4, clients connects on IPv6 -> situation 1 and 3 4O6O: situation 4 Situation 1: * The client appears to hang, because the netcat is not returning information. This situation should be avoided because it will give a bad user experience. Situation 2: * The server shows in its log: 'Error creating listening socket: Address already in use' * The server is not started This is intended behaviour Situation 3: * The server does not show a warning in its log, but it will log a warning to the console: 'WARNING **:Not using socket because bind failed: Address already in use' * The server is started I would prefer that the server is not started, and nothing it sent to the console but the message is sent to the log window. Situation 4: * The log gets a line as in situation 2. * Two warnings as in situation 3 are sent to the console. * When the 'Stop Server' button is pressed, another line is sent to the log window 'Preparing game...' followed by a line as in situation 2, and a warning as in situation 3 is sent to the console. It looks like the server is started twice, the first time with two protocols and the second time with one protocol. That looks like a bug. I would prefer a situation similar to situation 2. About the code: I would recommend to use GSList (or something similar) instead of int* and a counter variable. With kind regards, Roland Clobus
Index: common/network.h
===================================================================
--- common/network.h (revision 1493)
+++ common/network.h (working copy)
@@ -82,9 +82,11 @@
* @param port The port
* @retval error_message If opening fails, a description of the error
* You should g_free the error_message
- * @return A file descriptor if succesfull, or -1 if it fails
+ * @retval num_fds The number of valid fds in the returned array
+ * @return An array of file descriptors if succesfull, or NULL if it fails
*/
-int net_open_listening_socket(const gchar * port, gchar ** error_message);
+int *net_open_listening_socket(const gchar * port, gchar ** error_message,
+ int *num_fds);
/** Close a socket
*/
Index: common/network.c
===================================================================
--- common/network.c (revision 1493)
+++ common/network.c (working copy)
@@ -743,14 +743,18 @@
return pioneers_dir;
}
-int net_open_listening_socket(const gchar * port, gchar ** error_message)
+int *net_open_listening_socket(const gchar * port, gchar ** error_message,
+ int *num_fds)
{
#ifdef HAVE_GETADDRINFO_ET_AL
int err;
struct addrinfo hints, *ai, *aip;
int yes;
- gint fd = -1;
+ int *fds;
+ int max_num_fds;
+ *num_fds = 0;
+
memset(&hints, 0, sizeof(hints));
hints.ai_family = NETWORK_PROTOCOL;
@@ -763,65 +767,89 @@
g_strdup_printf(_(""
"Error creating struct addrinfo: %s"),
gai_strerror(err));
- return -1;
+ return NULL;
}
+ max_num_fds = 0;
for (aip = ai; aip; aip = aip->ai_next) {
- fd = socket(aip->ai_family, SOCK_STREAM, 0);
- if (fd < 0) {
+ ++max_num_fds;
+ }
+
+ fds = g_malloc(max_num_fds * sizeof(*fds));
+ for (aip = ai; aip; aip = aip->ai_next) {
+ fds[*num_fds] = socket(aip->ai_family, SOCK_STREAM, 0);
+ if (fds[*num_fds] < 0) {
continue;
}
- yes = 1;
/* setsockopt() before bind(); otherwise it has no effect! -- egnor */
+ yes = 1;
+ if ((aip->ai_family == AF_INET6)
+ &&
+ (setsockopt
+ (fds[*num_fds], IPPROTO_IPV6, IPV6_V6ONLY, &yes,
+ sizeof(yes)) < 0)) {
+ g_warning(_
+ (""
+ "Not using socket because setting IPV6_V6ONLY failed: %s\n"),
+ net_errormsg());
+ net_closesocket(fds[*num_fds]);
+ continue;
+ }
if (setsockopt
- (fd, SOL_SOCKET, SO_REUSEADDR, &yes,
+ (fds[*num_fds], SOL_SOCKET, SO_REUSEADDR, &yes,
sizeof(yes)) < 0) {
- net_closesocket(fd);
+ g_warning(_(""
+ "Not using socket because setting REUSEADDR failed: %s\n"),
+ net_errormsg());
+ net_closesocket(fds[*num_fds]);
continue;
}
- if (bind(fd, aip->ai_addr, aip->ai_addrlen) < 0) {
- net_closesocket(fd);
+ if (bind(fds[*num_fds], aip->ai_addr, aip->ai_addrlen) < 0) {
+ g_warning(_(""
+ "Not using socket because bind failed: %s\n"),
+ net_errormsg());
+ net_closesocket(fds[*num_fds]);
continue;
}
- break;
- }
+ if (net_set_socket_non_blocking(fds[*num_fds])) {
+ g_warning(_(""
+ "Not using socket because it cannot be set to non-blocking: %s\n"),
+ net_errormsg());
+ net_closesocket(fds[*num_fds]);
+ continue;
+ }
- if (!aip) {
- *error_message =
- g_strdup_printf(_(""
- "Error creating listening socket: %s\n"),
- net_errormsg());
- freeaddrinfo(ai);
- return -1;
+ if (listen(fds[*num_fds], 5) < 0) {
+ g_warning(_(""
+ "Not using socket because listening failed: %s\n"),
+ net_errormsg());
+ net_closesocket(fds[*num_fds]);
+ continue;
+ }
+
+ /* Listening succeeded. Keep the fd and continue. */
+ ++*num_fds;
}
freeaddrinfo(ai);
- if (net_set_socket_non_blocking(fd)) {
+ if (*num_fds == 0) {
+ g_free(fds);
*error_message =
g_strdup_printf(_(""
- "Error setting socket non-blocking: %s\n"),
+ "Error creating listening socket: %s\n"),
net_errormsg());
- net_closesocket(fd);
- return -1;
+ return NULL;
}
- if (listen(fd, 5) < 0) {
- *error_message =
- g_strdup_printf(_(""
- "Error during listen on socket: %s\n"),
- net_errormsg());
- net_closesocket(fd);
- return -1;
- }
*error_message = NULL;
- return fd;
+ return fds;
#else /* HAVE_GETADDRINFO_ET_AL */
*error_message =
g_strdup(_("Listening not yet supported on this platform."));
- return -1;
+ return NULL;
#endif /* HAVE_GETADDRINFO_ET_AL */
}
Index: meta-server/main.c
===================================================================
--- meta-server/main.c (revision 1493)
+++ meta-server/main.c (working copy)
@@ -92,7 +92,8 @@
static fd_set read_fds;
static fd_set write_fds;
-static int accept_fd;
+static int *accept_fds;
+static int num_accept_fds;
static gint max_fd;
/* Command line data */
@@ -189,8 +190,14 @@
static void find_new_max_fd(void)
{
GList *list;
+ int i;
- max_fd = accept_fd;
+ max_fd = -1;
+ for (i = 0; i < num_accept_fds; ++i) {
+ if (accept_fds[i] > max_fd) {
+ max_fd = accept_fds[i];
+ }
+ }
for (list = client_list; list != NULL; list = g_list_next(list)) {
Client *client = list->data;
@@ -836,7 +843,7 @@
set_client_event_at(client);
}
-static void accept_new_client(void)
+static void accept_new_client(int accept_fd)
{
int fd;
gchar *error_message;
@@ -928,6 +935,7 @@
fd_set read_res;
fd_set write_res;
int num;
+ int i;
struct timeval *timeout;
GList *list;
@@ -952,9 +960,11 @@
}
}
- if (FD_ISSET(accept_fd, &read_res)) {
- accept_new_client();
- num--;
+ for (i = 0; i < num_accept_fds; ++i) {
+ if (FD_ISSET(accept_fds[i], &read_res)) {
+ accept_new_client(accept_fds[i]);
+ num--;
+ }
}
list = client_list;
@@ -985,17 +995,22 @@
static gboolean setup_accept_sock(const gchar * port)
{
- int fd;
+ int *fds;
+ int i;
gchar *error_message;
- fd = net_open_listening_socket(port, &error_message);
- if (fd == -1) {
+ fds =
+ net_open_listening_socket(port, &error_message,
+ &num_accept_fds);
+ if (!fds) {
my_syslog(LOG_ERR, "%s", error_message);
g_free(error_message);
return FALSE;
}
- accept_fd = max_fd = fd;
- FD_SET(accept_fd, &read_fds);
+ accept_fds = fds;
+ find_new_max_fd();
+ for (i = 0; i < num_accept_fds; ++i)
+ FD_SET(accept_fds[i], &read_fds);
return TRUE;
}
@@ -1089,6 +1104,7 @@
GOptionContext *context;
GError *error = NULL;
GList *game_list;
+ int i;
setlocale(LC_ALL, "");
bindtextdomain(PACKAGE, LOCALEDIR);
@@ -1161,6 +1177,11 @@
my_syslog(LOG_INFO, "Pioneers meta server started.");
select_loop();
+ for (i = 0; i < num_accept_fds; ++i) {
+ net_closesocket(accept_fds[i]);
+ accept_fds[i] = -1;
+ }
net_finish();
+ g_free(accept_fds);
return 0;
}
Index: server/server.c
===================================================================
--- server/server.c (revision 1493)
+++ server/server.c (working copy)
@@ -74,8 +74,9 @@
game = g_malloc0(sizeof(*game));
- game->accept_tag = 0;
- game->accept_fd = -1;
+ game->accept_tag = NULL;
+ game->accept_fd = NULL;
+ game->num_fds = 0;
game->is_running = FALSE;
game->is_game_over = FALSE;
game->params = params_copy(params);
@@ -101,6 +102,10 @@
if (game->server_port != NULL)
g_free(game->server_port);
params_free(game->params);
+ if (game->accept_fd)
+ g_free(game->accept_fd);
+ if (game->accept_tag)
+ g_free(game->accept_tag);
g_free(game);
}
@@ -161,14 +166,16 @@
}
-static void player_connect(Game * game)
+static void player_connect(Connect_info * info)
{
gchar *location;
- gint fd = accept_connection(game->accept_fd, &location);
+ gint fd =
+ accept_connection(info->game->accept_fd[info->num], &location);
if (fd > 0) {
- if (player_new_connection(game, fd, location) != NULL)
- stop_timeout(game);
+ if (player_new_connection(info->game, fd, location) !=
+ NULL)
+ stop_timeout(info->game);
}
g_free(location);
}
@@ -177,21 +184,32 @@
const gchar * meta_server_name)
{
gchar *error_message;
+ int i;
game->accept_fd =
- net_open_listening_socket(game->server_port, &error_message);
- if (game->accept_fd == -1) {
+ net_open_listening_socket(game->server_port, &error_message,
+ &game->num_fds);
+ if (game->accept_fd == NULL) {
log_message(MSG_ERROR, "%s\n", error_message);
g_free(error_message);
return FALSE;
}
+ game->accept_tag =
+ g_malloc0(game->num_fds * sizeof(*game->accept_tag));
game->is_running = TRUE;
start_timeout(game);
- game->accept_tag = driver->input_add_read(game->accept_fd,
- (InputFunc)
- player_connect, game);
+ game->connect_info =
+ g_malloc0(game->num_fds * sizeof(Connect_info));
+ for (i = 0; i < game->num_fds; ++i) {
+ game->connect_info[i].game = game;
+ game->connect_info[i].num = i;
+ game->accept_tag[i] =
+ driver->input_add_read(game->accept_fd[i], (InputFunc)
+ player_connect,
+ &game->connect_info[i]);
+ }
if (register_server) {
g_assert(meta_server_name != NULL);
@@ -254,6 +272,7 @@
gboolean server_stop(Game * game)
{
GList *current;
+ int i;
if (!server_is_running(game))
return FALSE;
@@ -262,13 +281,18 @@
game->is_running = FALSE;
if (game->accept_tag) {
- driver->input_remove(game->accept_tag);
- game->accept_tag = 0;
+ for (i = 0; i < game->num_fds; ++i)
+ driver->input_remove(game->accept_tag[i]);
+ g_free(game->accept_tag);
+ game->accept_tag = NULL;
}
- if (game->accept_fd >= 0) {
- close(game->accept_fd);
- game->accept_fd = -1;
+ if (game->accept_fd) {
+ for (i = 0; i < game->num_fds; ++i)
+ close(game->accept_fd[i]);
+ g_free(game->accept_fd);
+ game->accept_fd = NULL;
}
+ game->num_fds = 0;
playerlist_inc_use_count(game);
current = game->player_list;
Index: server/server.h
===================================================================
--- server/server.h (revision 1493)
+++ server/server.h (working copy)
@@ -47,7 +47,13 @@
#define TERRAIN_RANDOM 1
typedef struct Game Game;
+
typedef struct {
+ Game *game; /* The game */
+ int num; /* The index into game->accept_fd */
+} Connect_info;
+
+typedef struct {
StateMachine *sm; /* state machine for this player */
Game *game; /* game that player belongs to */
@@ -89,8 +95,10 @@
GameParams *params; /* game parameters */
gchar *hostname; /* reported hostname */
- int accept_fd; /* socket for accepting new clients */
- int accept_tag; /* Gdk event tag for accept socket */
+ int num_fds; /* number of elements in accept_fd and accept_tag */
+ int *accept_fd; /* sockets for accepting new clients */
+ int *accept_tag; /* Gdk event tags for accept socket */
+ Connect_info *connect_info; /* Structure for finding the listening fd in the accept callback */
GList *player_list; /* all players in the game */
GList *dead_players; /* all players that should be removed when player_list_use_count == 0 */
Index: server/admin.c
===================================================================
--- server/admin.c (revision 1493)
+++ server/admin.c (working copy)
@@ -35,7 +35,8 @@
#include "server.h"
/* network administration functions */
-comm_info *_accept_info = NULL;
+static comm_info *_accept_info = NULL;
+static int num_accept_fds = 0;
gint admin_dice_roll = 0;
@@ -373,25 +374,35 @@
void admin_listen(const gchar * port)
{
gchar *error_message;
+ int num_fds;
+ int *fds;
+ int i;
- if (!_accept_info) {
- _accept_info = g_malloc0(sizeof(comm_info));
- }
-
- /* open up a socket on which to listen for connections */
- _accept_info->fd = net_open_listening_socket(port, &error_message);
- if (_accept_info->fd == -1) {
+ /* open up sockets on which to listen for connections */
+ fds = net_open_listening_socket(port, &error_message, &num_fds);
+ if (!fds) {
log_message(MSG_ERROR, "%s\n", error_message);
g_free(error_message);
return;
}
+
+ if (!_accept_info) {
+ _accept_info = g_malloc0(sizeof(comm_info) * num_fds);
+ } else if (num_accept_fds != num_fds) {
+ g_free(_accept_info);
+ _accept_info = g_malloc0(sizeof(comm_info) * num_fds);
+ }
+ num_accept_fds = num_fds;
+
+ for (i = 0; i < num_fds; ++i) {
#ifdef PRINT_INFO
- g_print("admin_listen: fd = %d\n", _accept_info->fd);
+ g_print("admin_listen: fd[%d] = %d\n", i, fds[i]);
#endif
-
- /* set up the callback to handle connections */
- _accept_info->read_tag =
- driver->input_add_read(_accept_info->fd,
- (InputFunc) admin_connect,
- _accept_info);
+ _accept_info[i].fd = fds[i];
+ /* set up the callback to handle connections */
+ _accept_info[i].read_tag =
+ driver->input_add_read(fds[i],
+ (InputFunc) admin_connect,
+ &_accept_info[i]);
+ }
}
Index: server/admin.h
===================================================================
--- server/admin.h (revision 1493)
+++ server/admin.h (working copy)
@@ -25,7 +25,7 @@
#include "network.h"
typedef struct _comm_info {
- gint fd;
+ int fd;
guint read_tag;
guint write_tag;
} comm_info;
signature.asc
Description: This is a digitally signed message part
