minor code refactoring after awesome first version

moxerver.c

@@ -9,11 +9,6 @@
 #define PORT_MAX 4008 /* maximum TCP port number */
 
 
-/* Global variables used throughout the application */
-//struct server_t server;
-//struct client_t client; //TODO working with only 1 client, this can be expanded into a list
-//struct tty_t tty_dev;
-
 /* Prints help message. */
 static void usage() {
 	//TODO maybe some styling should be done
@@ -24,10 +19,13 @@ static void usage() {
 /* Performs cleanup and exit. */
 void cleanup(int exit_code) {
 	fprintf(stderr, "[%s] cleanup and exit with %d\n", NAME, exit_code);
-	/* close client and server */
+	/* close client */
 	if (client.socket != -1) {
 		client_close(&client);
 	}
+	/* close tty device */
+	tty_close(&tty_dev);
+	/* close server */
 	server_close(&server);
 	exit(exit_code);
 }
@@ -77,7 +75,7 @@ int main(int argc, char *argv[]) {
 					usage();
 					return -1;
 				} else {
-					/* set tty device path to in tty_dev struct */
+					/* set tty device path in tty_dev struct */
 					strcpy(tty_dev.path, optarg);
 				}
 				break;
@@ -93,6 +91,7 @@ int main(int argc, char *argv[]) {
 	}
 	/* check arguments */
 	if (tcp_port < PORT_MIN || tcp_port > PORT_MAX) {
+		//TODO do we really put port constraints in moxerver? Maybe higher SW layer that controls all servers should handle it.
 		fprintf(stderr, "[%s] error: port number out of %d-%d range\n\n",
 				NAME, PORT_MIN, PORT_MAX);
 		usage();
@@ -109,15 +108,15 @@ int main(int argc, char *argv[]) {
 	server_setup(&server, tcp_port);
 	client.socket = -1;
 	
-	
-	//TODO this is a good place to create and start the TTY thread, use "tty_dev.path" when opening device
+	/* open tty device */
 	if (tty_open(&tty_dev) < 0) {
 		fprintf(stderr, "[%s] error: opening of tty device at %s failed\n"
 				"\t\t-> continuing in echo mode\n", NAME, tty_dev.path); 
 		//return -1;
 	}
-
-	ret = pthread_create(&tty_thread, NULL, tty_thread_func, &tty_dev);
+	
+	/* start thread that handles tty device */
+	ret = pthread_create(&tty_thread, NULL, tty_thread_func, &tty_dev); //TODO check return value?
 	
 	/* loop with timeouts waiting for client connection and data*/
 	while (1) {
@@ -163,7 +162,7 @@ int main(int argc, char *argv[]) {
 			if ( (client.socket != -1) && FD_ISSET(client.socket, &read_fds) ) {
 				/* read client data */
 				ret = client_read(&client);
-				/* check if client disconnected */
+				/* check if client disconnected or other errors occurred */
 				if (ret == -ENODATA) {
 					fprintf(stderr, "[%s] client %s disconnected\n", NAME, client.ip_string);
 					/* close client connection and continue waiting for new clients */
@@ -175,10 +174,7 @@ int main(int argc, char *argv[]) {
 					fprintf(stderr, "[%s] problem reading client\n", NAME);
 					continue;
 				}
-				/* echo back to client */
-				//client_write(&client, client.data, ret);
-				
-				//TODO we should send this data to TTY device here
+				/* pass received data to tty device */
 				tty_write(&tty_dev, client.data, ret); 
 			}
 		}
@@ -188,11 +184,10 @@ int main(int argc, char *argv[]) {
 		
 	} /* END while() loop */
 	
-	pthread_join(tty_thread, NULL);
-	
 	/* unexpected break from while() loop */
 	fprintf(stderr, "[%s] unexpected condition\n", NAME);
 	/* cleanup and exit with -1 */
+	pthread_join(tty_thread, NULL); //TODO maybe we should be able to kill this thread now, what if we wait forever?
 	cleanup(-1);
 	
 	return -1;

moxerver.h

@@ -34,9 +34,12 @@ struct tty_t {
 	char data[DATA_BUFLEN]; 	/* buffer for data received from tty */
 };
 
-struct server_t server;
-struct client_t client; //TODO working with only 1 client, this can be expanded into a list
-struct tty_t tty_dev;
+
+/* Global variables used throughout the application. */
+struct server_t server; 	/* main server structure */
+struct client_t client; 	/* connected client structure */ //TODO working with only 1 client, this can be expanded into a list
+struct tty_t tty_dev; 		/* connected tty device */
+
 
 /* Sets up the server on specific port, binds to a socket and listens for client connections. */
 int server_setup(struct server_t *server, unsigned int port);

telnet.c

@@ -17,9 +17,9 @@ struct telnet_option_t telnet_options[] = {
 	{"ECHO", 1},
 	{"SGA", 3},
 	{"LINEMODE", 34},
-	{"SLE", 45}
 };
-#define TELNET_OPTIONS_COUNT 9 /* keep this up with the number of supported options */
+#define TELNET_OPTIONS_COUNT 8 /* keep this up with the number of supported options */
+//TODO implement this list with last element being NULL so we don't need to keep count
 
 /* Returns telnet option name based on the value. */
 static const char* telnet_option_name(int value) {
@@ -40,7 +40,7 @@ static char telnet_option_value(const char* name) {
 }
 
 /* Sends telnet option command. */
-int telnet_send_command(struct client_t *client, const char* option, const char* command) {
+static int telnet_send_command(struct client_t *client, const char* option, const char* command) {
 	
 	char data[3];
 	
@@ -60,7 +60,7 @@ int telnet_send_command(struct client_t *client, const char* option, const char*
 }
 
 /* Handles received telnet option command. */
-int telnet_handle_command(char *databuf, int datalen) {
+static int telnet_handle_command(char *databuf, int datalen) {
 	
 	if (databuf[0] == telnet_option_value("IAC")) {
 		fprintf(stderr, "[%s] received %s %s\n", __func__,

tty.c

@@ -1,5 +1,5 @@
 #include "moxerver.h"
-#include <string.h>
+
 #define TTY_THREAD_TIMEOUT_SEC 30
 #define TTY_WAIT_TIMEOUT 5 /* seconds for select() timeout in server loop */
 #define NAME "tty"