cleanup&refactor sock.c a bit, change definition for client add/remove functions

server/clients.c

@@ -1,6 +1,9 @@
-/*
+/** \file clients.c
- * clients.c
+ * Manage the list of clients that are connected.
- * This file is part of LCDd, the lcdproc server.
+ * Init/shut down client system, and search for clients in the list.
+ */
+
+/* This file is part of LCDd, the lcdproc server.
  *
  * This file is released under the GNU General Public License. Refer to the
  * COPYING file distributed with this package.
@@ -8,11 +11,6 @@
  * Copyright (c) 1999, William Ferrell, Scott Scriven
  *               2002, Joris Robijn
  *
- *
- * Inits/shuts down client system,
- * and searches for clients in the list.
- * On short: manages the list of clients that are connected.
- *
  */
 
 #include <stdio.h>
@@ -77,18 +75,23 @@ clients_shutdown(void)
 	return 0;
 }
 
-int
+/* Add the client to the clients list... */
+Client *
 clients_add_client(Client *c)
 {
-	/* Add the client to the clients list... */
+	if (LL_Push(clientlist, c) == 0)
-	return LL_Push(clientlist, c);
+		return c;
+
+	return NULL;
 }
 
-int
+/* Remove the client from the clients list... */
+Client *
 clients_remove_client(Client *c)
 {
-	/* Remove the client from the clients list... */
+	Client *client = LL_Remove(clientlist, c);
-	return(LL_Remove(clientlist, c) == NULL)?-1:0;
+	
+	return client;
 }
 
 Client *
@@ -113,7 +116,6 @@ clients_client_count(void)
 /* A client is identified by the file descriptor
  * associated with it. Find one.
  */
-
 Client *
 clients_find_client_by_sock(int sock)
 {

server/clients.h

@@ -1,6 +1,8 @@
-/*
+/** \file clients.h
- * client.h
+ * Manage the list of clients that are connected.
- * This file is part of LCDd, the lcdproc server.
+ */
+
+/* This file is part of LCDd, the lcdproc server.
  *
  * This file is released under the GNU General Public License. Refer to the
  * COPYING file distributed with this package.
@@ -21,9 +23,9 @@
 int clients_init(void);
 int clients_shutdown(void);
 
-/* Add/remove clients (return -1 for error) */
+/* Add/remove clients (return NULL for error) */
-int clients_add_client(Client *c);
+Client *clients_add_client(Client *c);
-int clients_remove_client(Client *c);
+Client *clients_remove_client(Client *c);
 
 /* List functions */
 Client *clients_getfirst(void);

server/sock.c

@@ -10,7 +10,7 @@
  * Copyright (c) 1999, William Ferrell, Scott Scriven
  *               2003, Benjamin Tse (blt@ieee.org) - Winsock port
  *               2004, F5 Networks, Inc. - IP-address input
- *               2005, Peter Marschall - error checks, ...
+ *               2005-2008, Peter Marschall - error checks, ...
  *
  */
 
@@ -57,21 +57,25 @@ static int listening_fd;
 static LinkedList* openSocketList = NULL;
 static LinkedList* freeClientSocketList = NULL;
 
-struct ClientSocketMap
+/** Mapping between socket and associated client */
+typedef struct _ClientSocketMap
 {
-    int socket;
+	int socket;		/**< Socket for the client */
-    Client* client;
+	Client *client;		/**< Pointer to client representation */
-};
+} ClientSocketMap;
+
+
+/* The memory referenced from \c openSocketList and \c freeSocketList
+ * is obtained from the freeClientSocketPool array. */
+ClientSocketMap *freeClientSocketPool;
 
-/* The memory referenced from clientSocketPoolList is obtained from the
- * clientSocketPool array. */
-struct ClientSocketMap* freeClientSocketPool;
 
 /* Length of longest transmission allowed at once...*/
 #define MAXMSG 8192
 
 /**** Internal function declarations ****************************************/
-static int sock_read_from_client(struct ClientSocketMap* clientSocketMap);
+static int sock_read_from_client(ClientSocketMap *clientSocketMap);
+static void sock_destroy_socket(void);
 
 
 /** Initialize sockets.
@@ -90,8 +94,7 @@ sock_init(char* bind_addr, int bind_port)
 	/* Initialize the Winsock dll */
 	WSADATA wsaData;
 	int startup = WSAStartup(MAKEWORD(2, 2), &wsaData);
-        if (startup != 0)
+	if (startup != 0) {
-        {
 		report(RPT_ERR, "%s: Could not start Winsock library - %s", 
 			__FUNCTION__, sock_geterror());
 	}
@@ -103,7 +106,7 @@ sock_init(char* bind_addr, int bind_port)
 	/* Create the socket and set it up to accept connections. */
 	listening_fd = sock_create_inet_socket(bind_addr, bind_port);
 	if (listening_fd < 0) {
-		report(RPT_ERR, "%s: Error creating socket - %s", 
+		report(RPT_ERR, "%s: error creating socket - %s", 
 			__FUNCTION__, sock_geterror());
 		return -1;
 	}
@@ -111,32 +114,35 @@ sock_init(char* bind_addr, int bind_port)
 	/* Create the socket -> Client mapping pool */
 	/* How large can FD_SETSIZE be? Even if it is ~2000 this only uses a
 	   few kilobytes of memory. Let's trade size for speed! */
-        freeClientSocketPool = (struct ClientSocketMap*) 
+	freeClientSocketPool = (ClientSocketMap *) 
-                        malloc(sizeof(struct ClientSocketMap) * FD_SETSIZE);
+				calloc(FD_SETSIZE, sizeof(ClientSocketMap));
-        if (!freeClientSocketPool)
+	if (freeClientSocketPool == NULL) {
-        {
+		report(RPT_ERR, "%s: Error allocating client sockets.",
-            report(RPT_ERR, "%s: Error allocating memory for client sockets.",
 			__FUNCTION__);
 		return -1;
 	}
 
 	freeClientSocketList = LL_new();
-        for (i = 0; i < FD_SETSIZE; ++i)
+	if (freeClientSocketList == NULL) {
-        {
+		report(RPT_ERR, "%s: error allocating free socket list.",
+			 __FUNCTION__);
+		return -1;
+	}
+	for (i = 0; i < FD_SETSIZE; ++i) {
 		LL_AddNode(freeClientSocketList, (void*) &freeClientSocketPool[i]);
 	}
 
 	/* Create and initialize the open socket list with the server socket */
 	openSocketList = LL_new();
-        if (!openSocketList)
+	if (openSocketList == NULL) {
-        {
+		report(RPT_ERR, "%s: error allocating open socket list.",
-            report(RPT_ERR, "%s: Error allocating memory for the open socket "
+			 __FUNCTION__);
-                   "list.", __FUNCTION__);
 		return -1;
-        } else {
+	}
-            struct ClientSocketMap *entry;
+	else {
+		ClientSocketMap *entry;
 
-            entry = (struct ClientSocketMap*) LL_Pop(freeClientSocketList);
+		entry = (ClientSocketMap*) LL_Pop(freeClientSocketList);
 		entry->socket = listening_fd;
 		entry->client = NULL;
 		LL_AddNode(openSocketList, (void*) entry);
@@ -173,13 +179,13 @@ sock_shutdown(void)
 
 	debug(RPT_DEBUG, "%s()", __FUNCTION__);
 
-        /*struct ClientSocketMap* clientIt;*/
+        /*ClientSocketMap* clientIt;*/
 
         /* delete all clients */
         /* This should be done by calling clients_shutdown */
         /*
           LL_Rewind(openSocketList);
-          for (clientIt = (struct ClientSocketMap*) LL_Get(openSocketList);
+          for (clientIt = (ClientSocketMap*) LL_Get(openSocketList);
           clientIt;
           clientIt = LL_GetNext(openSocketList))
           {
@@ -197,8 +203,7 @@ sock_shutdown(void)
 	free(freeClientSocketPool);
 
 #ifdef WINSOCK2
-        if (WSACleanup() != 0)
+	if (WSACleanup() != 0) {
-        {
 		report(RPT_ERR, "%s: Error closing Winsock library - %s",
 			__FUNCTION__, sock_geterror());
 		retVal = -1;
@@ -219,7 +224,8 @@ int
 sock_create_inet_socket(char *addr, unsigned int port)
 {
 	struct sockaddr_in name;
-	int sock, sockopt=1;
+	int sock;
+	int sockopt = 1;
 
 	debug(RPT_DEBUG, "%s(addr=\"%s\", port=%i)", __FUNCTION__, addr, port);
 
@@ -231,13 +237,13 @@ sock_create_inet_socket(char *addr, unsigned int port)
 	if (sock < 0)
 #endif
 	{
-		report(RPT_ERR, "%s: Could not create socket - %s", 
+		report(RPT_ERR, "%s: cannot create socket - %s", 
 			__FUNCTION__, sock_geterror());
 		return -1;
 	}
 	/* Set the socket so we can re-use it*/
-	if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, (void*)&sockopt, sizeof(sockopt)) < 0) {
+	if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, (void *) &sockopt, sizeof(sockopt)) < 0) {
-		report(RPT_ERR, "%s: Error setting socket option SO_REUSEADDR - %s", 
+		report(RPT_ERR, "%s: error setting socket option SO_REUSEADDR - %s", 
 			__FUNCTION__, sock_geterror());
 		return -1;
 	}
@@ -254,13 +260,10 @@ sock_create_inet_socket(char *addr, unsigned int port)
 #endif
 
 	if (bind(sock, (struct sockaddr *) &name, sizeof(name)) < 0) {
-		report(RPT_ERR, "%s: Could not bind to port %d at address %s - %s", 
+		report(RPT_ERR, "%s: cannot bind to port %d at address %s - %s", 
                        __FUNCTION__, port, addr, sock_geterror());
 		return -1;
 	}
-	else {
-		report(RPT_NOTICE, "Listening for queries on %s:%d", addr, port);
-	}
 
 	if (listen(sock, 1) < 0) {
 		report(RPT_ERR, "%s: error in attempting to listen to port "
@@ -269,6 +272,8 @@ sock_create_inet_socket(char *addr, unsigned int port)
 		return -1;
 	}
 
+	report(RPT_NOTICE, "Listening for queries on %s:%d", addr, port);
+
 	/* Initialize the set of active sockets. */
 	FD_ZERO(&active_fd_set);
 	FD_SET(sock, &active_fd_set);
@@ -285,7 +290,7 @@ int
 sock_poll_clients(void)
 {
 	struct timeval t;
-        struct ClientSocketMap* clientSocket;
+	ClientSocketMap* clientSocket;
 
 	debug(RPT_DEBUG, "%s()", __FUNCTION__);
 
@@ -303,7 +308,7 @@ sock_poll_clients(void)
 
 	/* Service all the sockets with input pending. */
 	LL_Rewind(openSocketList);
-        for (clientSocket = (struct ClientSocketMap *) LL_Get(openSocketList); 
+	for (clientSocket = (ClientSocketMap *) LL_Get(openSocketList); 
 	     clientSocket != NULL; 
 	     clientSocket = LL_GetNext(openSocketList)) {
 
@@ -317,11 +322,10 @@ sock_poll_clients(void)
 
 				new_sock = accept(listening_fd, (struct sockaddr *) &clientname, &size);
 #ifdef WINSOCK2
-                                if (new_sock == INVALID_SOCKET)
+				if (new_sock == INVALID_SOCKET) {
 #else
-				if (new_sock < 0) 
+				if (new_sock < 0) {
 #endif
-                                {
 					report(RPT_ERR, "%s: Accept error - %s", 
 						__FUNCTION__, sock_geterror());
 					return -1;
@@ -347,12 +351,12 @@ sock_poll_clients(void)
 				}
 				else {
 					/* add new_sock */
-                                        struct ClientSocketMap *newClientSocket;
+					ClientSocketMap *newClientSocket;
-                                        newClientSocket = (struct ClientSocketMap *) LL_Pop(freeClientSocketList);
+					newClientSocket = (ClientSocketMap *) LL_Pop(freeClientSocketList);
 					if (newClientSocket != NULL) {
 						newClientSocket->socket = new_sock;
 						newClientSocket->client = c;
-                                            LL_InsertNode(openSocketList, (void*) newClientSocket);
+						LL_InsertNode(openSocketList, (void *) newClientSocket);
 						/* advance past the new node - check it on the next pass */
 						LL_Next(openSocketList);
 					}
@@ -362,8 +366,9 @@ sock_poll_clients(void)
 						return -1;
 					}
 				}
-				if (clients_add_client(c) != 0) {
+				if (clients_add_client(c) == NULL) {
-					report(RPT_ERR, "%s: Could not add client on socket %i", __FUNCTION__, clientSocket->socket);
+					report(RPT_ERR, "%s: Could not add client on socket %i",
+						 __FUNCTION__, clientSocket->socket);
 					return -1;
 				}
 			}
@@ -374,27 +379,8 @@ sock_poll_clients(void)
 					debug(RPT_DEBUG, "%s: reading...", __FUNCTION__);
 					err = sock_read_from_client(clientSocket);
 					debug(RPT_DEBUG, "%s: ...done", __FUNCTION__);
-					if (err < 0) {
+					if (err < 0)
-                                            /* Client disconnected, destroy client data */
+						sock_destroy_socket();
-                                            if (clientSocket->client) {
-                                                struct ClientSocketMap *entry;
-
-                                                /*sock_send_string(i, "bye\n");*/
-                                                report(RPT_NOTICE, "Client on socket %i disconnected",
-                                                        clientSocket->socket);
-                                                client_destroy(clientSocket->client);
-                                                clients_remove_client(clientSocket->client);
-                                                FD_CLR(clientSocket->socket, &active_fd_set);
-                                                close(clientSocket->socket);
-
-                                                entry = (struct ClientSocketMap *) LL_DeleteNode(openSocketList);
-                                                LL_Push(freeClientSocketList, (void*) entry);
-                                            }
-					    else {
-                                                report(RPT_ERR, "%s: Can't find client of socket %i",
-                                                        __FUNCTION__, clientSocket->socket);
-                                            }
-					}
 				} while (err > 0);
 			}
 		}
@@ -408,7 +394,7 @@ sock_poll_clients(void)
  * \retval   0       success
  */
 static int
-sock_read_from_client(struct ClientSocketMap *clientSocketMap)
+sock_read_from_client(ClientSocketMap *clientSocketMap)
 {
 	char buffer[MAXMSG];
 	int nbytes, i;
@@ -452,6 +438,64 @@ sock_read_from_client(struct ClientSocketMap *clientSocketMap)
 }
 
 
+/* comparison function to find a ClientsocketMap entry by client */
+int byClient(void *csm, void *client)
+{
+	return (((ClientSocketMap *) csm)->client == (Client *) client) ? 0 : -1;
+}
+
+
+/** Close an open socket for a given client.
+ * \param client  Client whose socket shall be closed.
+ * \retval <0     error
+ * \retval  0     success.
+ */
+int
+sock_destroy_client_socket(Client *client)
+{
+	ClientSocketMap *entry;
+
+	LL_Rewind(openSocketList);
+	entry = LL_Find(openSocketList, byClient, client);
+
+	if (entry != NULL) {
+		sock_destroy_socket();
+		return 0;
+	}
+	return -1;
+}
+
+
+/** Close the socket the openSocketList's \c current pointer points to.
+ */
+static void
+sock_destroy_socket(void)
+{
+	ClientSocketMap *entry = LL_Get(openSocketList);
+
+	if (entry != NULL) {
+		if (entry->client != NULL) {
+			report(RPT_NOTICE, "Client on socket %i disconnected",
+				entry->socket);
+			client_destroy(entry->client);
+			clients_remove_client(entry->client);
+			entry->client = NULL;
+		}
+		else {
+			report(RPT_ERR, "%s: Can't find client of socket %i",
+				__FUNCTION__, entry->socket);
+		}
+		/* close socket and remove it from select()'s mask of active sockets */
+		FD_CLR(entry->socket, &active_fd_set);
+		close(entry->socket);
+
+		/* re-add socket to the free socket pool */
+		entry = (ClientSocketMap *) LL_DeleteNode(openSocketList);
+		LL_Push(freeClientSocketList, (void*) entry);
+	}
+}
+
+
 /* return 1 if addr is valid IPv4 */
 int verify_ipv4(const char *addr) 
 {

server/sock.h

@@ -1,13 +1,15 @@
-/*
+/** \file sock.h
- * sock.h
+ * function declarations for LCDproc sockets code
- * This file is part of LCDd, the lcdproc server.
+ */
+
+/* This file is part of LCDd, the lcdproc server.
  *
  * This file is released under the GNU General Public License. Refer to the
  * COPYING file distributed with this package.
  *
  * Copyright (c) 1999, William Ferrell, Scott Scriven
  *               2004, F5 Networks, Inc. - IP-address verification
- *
+ *               2008, Peter Marschall
  */
 
 #ifndef SOCK_H
@@ -15,13 +17,14 @@
 
 #include "shared/sockets.h"
 
-typedef struct sockaddr_in sockaddr_in;
+#include "client.h"
 
 /* Server functions...*/
 int sock_init(char* bind_addr, int bind_port);
 int sock_shutdown(void);
 int sock_create_inet_socket(char* bind_addr, unsigned int port);
 int sock_poll_clients(void);
+int sock_destroy_client_socket(Client *client);
 int verify_ipv4(const char *addr);
 int verify_ipv6(const char *addr);