remove unused code, cleanup & shorten parse_message() function;

hide commands[] table; make it's contents accessible using a function

server/commands/command_list.c

@@ -25,31 +25,49 @@
 #include "menu_commands.h"
 
 #include <stdlib.h>
+#include <string.h>
 
-client_function commands[] = {
+static client_function commands[] = {
-	{"test_func", test_func_func},
+	{ "test_func",      test_func_func      },
-	{"hello", hello_func},
+	{ "hello",          hello_func          },
-	{"client_set", client_set_func},
+	{ "client_set",     client_set_func     },
-	{"client_add_key", client_add_key_func},
+	{ "client_add_key", client_add_key_func },
-	{"client_del_key", client_del_key_func},
+	{ "client_del_key", client_del_key_func },
-/*	{"screen_add_key", screen_add_key_func}, */
+/*	{ "screen_add_key", screen_add_key_func }, */
-/*	{"screen_del_key", screen_del_key_func}, */
+/*	{ "screen_del_key", screen_del_key_func }, */
-	{"screen_add", screen_add_func},
+	{ "screen_add",     screen_add_func     },
-	{"screen_del", screen_del_func},
+	{ "screen_del",     screen_del_func     },
-	{"screen_set", screen_set_func},
+	{ "screen_set",     screen_set_func     },
-	{"widget_add", widget_add_func},
+	{ "widget_add",     widget_add_func     },
-	{"widget_del", widget_del_func},
+	{ "widget_del",     widget_del_func     },
-	{"widget_set", widget_set_func},
+	{ "widget_set",     widget_set_func     },
-	{"menu_add_item", menu_add_item_func},
+	{ "menu_add_item",  menu_add_item_func  },
-	{"menu_del_item", menu_del_item_func},
+	{ "menu_del_item",  menu_del_item_func  },
-	{"menu_set_item", menu_set_item_func},
+	{ "menu_set_item",  menu_set_item_func  },
-	{"menu_goto", menu_goto_func},
+	{ "menu_goto",      menu_goto_func      },
-	{"menu_set_main", menu_set_main_func},
+	{ "menu_set_main",  menu_set_main_func  },
 	/* Misc stuff...*/
-	{"backlight", backlight_func},
+	{ "backlight",      backlight_func      },
-	{"output", output_func},
+	{ "output",         output_func         },
-	{"noop", noop_func},
+	{ "noop",           noop_func           },
-	{"info", info_func},
+	{ "info",           info_func           },
-	{"sleep", sleep_func},
+	{ "sleep",          sleep_func          },
-	{NULL, NULL},
+	{ NULL,             NULL},
 };
+
+
+CommandFunc get_command_function(char *cmd)
+{
+	int i;
+
+	if (cmd == NULL)
+		return NULL;
+
+	for (i = 0; commands[i].keyword != NULL; i++) {
+		if (0 == strcmp(cmd, commands[i].keyword))
+			return commands[i].function;
+	}
+
+	return NULL;
+}
+

server/commands/command_list.h

@@ -19,13 +19,14 @@
   point to a function to call, defined below.
  */
 
+typedef int (*CommandFunc) (Client * c, int argc, char **argv);
+
 typedef struct client_function {
 	char *keyword;
-	int (*function) (Client * c, int argc, char **argv);
+	CommandFunc function;
 } client_function;
 
-/* FIXME?  Do these really need to be visible from other sources?*/
 
-extern client_function commands[];
+CommandFunc get_command_function(char *cmd);
 
 #endif

server/parse.c

@@ -27,177 +27,26 @@
 #include "commands/command_list.h"
 #include "parse.h"
 
-/* This is a big function... TOO big.  How to trim....
- * TODO: Simplify... simplify...
- */
 
 #define MAX_ARGUMENTS 40
 
-int
-parse_all_client_messages2 ()
-{
-	int i; /* int j, len;*/
-	/*int newtoken, inquote;*/
-	Client * c;
-	char *str, *p, *q, *s;
-/*   char *tok;*/
-	int argc;
-	char *argv[MAX_ARGUMENTS];
-	/*char delimiters[] = " ";*/
-	char leftquote[] = "\"'`([{";
-	char rightquote[] = "\"'`)]}";
-	char errmsg[256];
-	int invalid = 0;
-	int quoteindex;
-
-	debug( RPT_DEBUG, "parse_all_client_messages()" );
-
-	for (i = 0; i < MAX_ARGUMENTS; i++) {
-		argv[i] = NULL;
-	}
-
-#define	SEPARATOR_CHAR	' '
-#define	LINE_TERM_CHAR	'\0'
-#define COMMENT_CHAR '#'
-
-	for( c=clients_getfirst(); c; c=clients_getnext()) {
-
-		/* And parse all its messages...*/
-		/*debug(RPT_DEBUG, "parse: Getting messages...");*/
-		for (str = client_get_message (c); str; str = client_get_message (c)) {
-
-			debug (RPT_DEBUG, "parse: ...%s", str);
-			/* Now, split up the string...*/
-			argc = 0;
-			i = 0;
-			q = p = str;
-
-			if (*p == COMMENT_CHAR) {
-				continue; /* found a comment line - skip it...*/
-			}
-
-			debug (RPT_DEBUG, "starting string scan...");
-
-			do {
-				/* bypass initial white space...*/
-				while ((*p == SEPARATOR_CHAR) && (*p)) {
-					p++;
-					q++;
-				}
-
-				/* If (*p) is null here, we reached the end of
-				 * an empty parameter... so one of two things
-				 * is true:
-				 *
-				 * 1. There is nothing but white space on this line (odd..)
-				 * 2. This is trailing white space (odd... but allowable)
-				 */
-
-				if (*p == LINE_TERM_CHAR) {
-					break;
-					/* if there are no arguments, argc == 0 and will fail
-					 * appropriately...
-					 *
-					 * if this is trailing white space, ignore the argc++ at the
-					 * end and claim this as the end...
-					 */
-				}
-
-				/* Handle quoted strings...*/
-				if ((s = strchr(leftquote, *p)) != NULL) {
-					quoteindex = s - leftquote;
-					/*debug(RPT_DEBUG, "found <%c> at index [%d] = <%c>", *p, quoteindex, leftquote[quoteindex]);*/
-					q = ++p; /* past open quote...*/
-					while ((rightquote[quoteindex] != *p) && (*p != LINE_TERM_CHAR)) {
-						p++;
-					}
-					if (*p == LINE_TERM_CHAR) {
-						/* We just sucked up the rest of the command line: ERROR!!*/
-						snprintf (errmsg, sizeof(errmsg), "huh? unterminated string! missing ending %c\n",
-							rightquote[quoteindex]);
-						sock_send_string (c->sock, errmsg);
-						continue;
-					} else {
-						*p = LINE_TERM_CHAR;	/* terminate string*/
-						p++;		/* bypass to next character*/
-						/* Note that next character could be a EndOfLine (null)
-						 * if the string was last on the line, or it could be
-						 * something else... is it a blank?
-						 */
-						if (*p != SEPARATOR_CHAR && *p != LINE_TERM_CHAR) {
-							sock_send_string (c->sock, "huh? improperly terminated string! (missing whitespace)\n");
-							continue;
-						}
-					}
-
-				/* Otherwise, normal string...*/
-				} else {
-					while (*p != SEPARATOR_CHAR && *p != LINE_TERM_CHAR)
-						p++;
-				}
-
-				/* Not end of line?*/
-				if (*p) {
-					*p = LINE_TERM_CHAR;
-					/*debug(RPT_DEBUG, "found new token: %s", q);*/
-					argv[i++] = q;
-					q = ++p;
-				} else {
-					/*debug(RPT_DEBUG, "found new token: %s", q);*/
-					argv[i++] = q;
-				}
-				/* At the end of this statement,
-				 * *p will be '\0' if end of input reached;
-				 * otherwise, it is the first character of the
-				 * next part of the string.
-				 */
 
 
-				argc++;
+static int parse_message (const char *str, Client *c);
-			} while (*p && i < MAX_ARGUMENTS - 1);
-
-			/*debug(RPT_DEBUG, "exiting string scan...");*/
-
-			argv[argc] = NULL;
-			if (argc < 1)
-				continue;
-
-			/* Now find and call the appropriate function...*/
-			invalid = 1;
-			for (i = 0; commands[i].keyword; i++) {
-				if (0 == strcmp (argv[0], commands[i].keyword)) {
-					invalid = commands[i].function (c, argc, argv);
-					break; /* found our function - don't continue on...*/
-				}
-			}
-
-			if (invalid) {
-				snprintf (errmsg, sizeof(errmsg), "huh? Invalid command \"%.40s\"\n", argv[0]);
-				sock_send_string (c->sock, errmsg);
-			}
-
-			free (str);			  /* fixed memory leak?*/
-		}							  /* end for(str...)*/
-	}
-
-	return 0;
-}
-
-int parse_message (char * str, Client * c);
 
 int
 parse_all_client_messages ()
 {
 	Client * c;
-	char * str;
 
 	debug( RPT_DEBUG, "%s()", __FUNCTION__ );
 
-	for( c=clients_getfirst(); c; c=clients_getnext()) {
+	for (c = clients_getfirst(); c != NULL; c = clients_getnext()) {
+		char * str;
 
 		/* And parse all its messages...*/
 		/*debug(RPT_DEBUG, "parse: Getting messages...");*/
-		for (str = client_get_message (c); str; str = client_get_message (c)) {
+		for (str = client_get_message (c); str != NULL; str = client_get_message (c)) {
 			parse_message (str, c);
 			free (str);
 		}
@@ -205,132 +54,111 @@ parse_all_client_messages ()
 	return 0;
 }
 
-int parse_message (char * str, Client * c)
+#define is_whitespace(x)	(((x) == ' ') || ((x) == '\t') || ((x) == '\r'))
+#define is_final(x)		(((x) == '\n') || ((x) == '\0'))
+#define is_opening_quote(x,q)   (((q) == '\0') && (((x) == '\"') || ((x) == '{')))
+#define is_closing_quote(x,q)   ((((q) == '{') && ((x) == '}')) || (((q) == '\"') && ((x) == '\"')))
+
+static int parse_message (const char *str, Client *c)
 {
-	typedef enum { ST_INITIAL, ST_WHITESPACE, ST_IGNORE, ST_ARGUMENT, ST_FINAL } State;
+	typedef enum { ST_INITIAL, ST_WHITESPACE, ST_ARGUMENT, ST_FINAL } State;
 	State state = ST_INITIAL;
 
-	char escape_chars[] = "nrt";
-	char escape_trans[] = "\n\r\t";
 	char errmsg[256];
 	int error = 0;
-	char used_quote = 0;	/* The quote used to open a quote string */
+	char quote = '\0';	/* The quote used to open a quote string */
 	int pos = 0;
-	char ch;
+	char *arg_space;
-	int i;
+	int argc = 0;
-	char * arg_space;
-	int argc =0 ;
 	char *argv[MAX_ARGUMENTS];
 	int argpos = 0;
 
-	void close_arg () {
+	void close_arg() {
-		if( argc == MAX_ARGUMENTS-1 ) {
+		if (argc >= MAX_ARGUMENTS-1) {
 			error = 1;
 		}
 		else {
-			argv[argc][argpos] = 0;
+			argv[argc][argpos] = '\0';
 			argv[argc+1] = argv[argc] + argpos + 1;
-			argc ++;
+			argc++;
 			argpos = 0;
 		}
 	}
 
 	debug( RPT_DEBUG, "%s( str=\"%.120s\", client=[%d] )", __FUNCTION__, str, c->sock );
 
-	arg_space = malloc(strlen(str)+1);
+	/* We will create a list of strings that is shorter or equally long as
-	argv[0] = arg_space;
-	/* We will create a new string that is shorter or equally long as
 	 * the original string str.
 	 */
+	arg_space = malloc(strlen(str)+1);
+	if (arg_space == NULL) {
+		report (RPT_ERR, "%s: Could not allocate memory", __FUNCTION__);
+		sock_send_string(c->sock, "huh? error allocating memory!\n");
+	}
 
-	while( state != ST_FINAL && !error ) {
+	argv[0] = arg_space;
-		ch = str[pos++];
-		switch( state ) {
 
+	while ((state != ST_FINAL) && !error) {
+		char ch = str[pos++];
+
+		switch (state) {
 		  case ST_INITIAL:
 		  case ST_WHITESPACE:
-			switch( ch ) {
+			if (is_whitespace(ch))
-			  case '\r':
-			  case '\t':
-			  case ' ':
 				break;
-			  case '\n':
+			if (is_final(ch)) {
-			  case 0:
 				state = ST_FINAL;
 				break;
-			  default:
+			}	  
+			/* otherwise fall through */
 			state = ST_ARGUMENT;
-				pos --; /* Rescan current char */
-			}
-			break;
-		  case ST_IGNORE:
-			switch( ch ) {
-			  case '\n':
-				state = ST_FINAL;
-			  	break;
-			}
-			break;
 		  case ST_ARGUMENT:
-			switch( ch ) {
+			if (is_final(ch)) {
-			  case '\r':
+				if (quote)
-			  case '\t':
-			  case ' ':
-				if (used_quote) {
-					/* We're in a quoted string, add it */
-					argv[argc][argpos++] = ch;
-				}
-				else {
-					close_arg();
-					state = ST_WHITESPACE;
-				}
-				break;
-			  case '\n':
-			  case 0:
-				if (used_quote) {
 					error = 2;
-				}
 				close_arg();
 				state = ST_FINAL;
-				break;
+			}
-			  case '\\':
+			else if (ch == '\\') {
 			 	if (str[pos]) {
 			 		/* We solve quoted chars here right away */
-			 		char * p;
+					const char escape_chars[] = "nrt";
-					p = strchr( escape_chars, str[pos] );
+					const char escape_trans[] = "\n\r\t";
+			 		char *p = strchr( escape_chars, str[pos] );
+					
+					/* Is it wise to have the characters \n, \r & \t expanded ?
+					 * Can the displays deal with them ?
+					 */
 					if (p != NULL) {
 						/* Insert a replacement for the code */
-						argv[argc][argpos++] = escape_trans[(int)*p];
+						argv[argc][argpos++] = escape_trans[p - escape_chars];
 					}
 					else {
-						 /* Copy char litterally */
+						 /* Copy char literally */
 						argv[argc][argpos++] = str[pos];
 					}
 					pos++;
 			 	}
 			 	else {
-			 		close_arg();
 			 		error = 2;
+					/* alternative: argv[argc][argpos++] = ch; */
+			 		close_arg();
 			 		state = ST_FINAL;
 			 	}
-			 	break;
-			  case '\"':
-			  case '{':
-				if (!used_quote) {
-					used_quote = ch;
-					break;
 			}
-				/* else fall through to default... */
+			else if (is_opening_quote(ch, quote)) {
-			  default:
+				quote = ch;
-				if (used_quote) {
-					/* Have we reached the end of the
-					 * quote already ?
-					 */
-					if ((used_quote == '{' && ch == '}')
-					|| (used_quote == '\"' && ch == '\"')) {
-						used_quote = 0;
-						break;
 			}	
+			else if (is_closing_quote(ch, quote)) {
+				quote = '\0';
+				close_arg();
+				state = ST_WHITESPACE;
 			}
+			else if (is_whitespace(ch) && (quote == '\0')) {
+				close_arg();
+				state = ST_WHITESPACE;
+			}	
+			else {
 				argv[argc][argpos++] = ch;
 			}	
 			break;
@@ -339,35 +167,37 @@ int parse_message (char * str, Client * c)
 			break;
 		}
 	}
+	if (argc < MAX_ARGUMENTS)
 		argv[argc] = NULL;
+	else
+		error = 1;
+
 	if (error) {
-		snprintf (errmsg, sizeof(errmsg), "huh? Could not parse command\n");
 		report( RPT_WARNING, "Could not parse command from client on socket %d: %.40s", c->sock, str );
+		snprintf (errmsg, sizeof(errmsg), "huh? Could not parse command\n");
 		sock_send_string (c->sock, errmsg);
 		free( arg_space  );
 		return 0;
 	}
 
 	/* Now find and call the appropriate function...*/
-	error = 1;
+	CommandFunc function = get_command_function(argv[0]);
-	for (i = 0; commands[i].keyword; i++) {
-		if (0 == strcmp (argv[0], commands[i].keyword)) {
-			error = commands[i].function (c, argc, argv);
-			break; /* found our function - don't continue on...*/
-		}
-	}
 
-	if (error == 1) {
+	if (function != NULL) {
-		snprintf (errmsg, sizeof(errmsg), "huh? Invalid command \"%.40s\"\n", argv[0]);
+		error = function (c, argc, argv);
-		sock_send_string (c->sock, errmsg);
+		if (error) {
-		report( RPT_WARNING, "Invalid command from client on socket %d: %.40s", c->sock, str );
-	}
-	else if (error) {
 			snprintf (errmsg, sizeof(errmsg), "huh? Function returned error \"%.40s\"\n", argv[0]);
 			sock_send_string (c->sock, errmsg);
 			report( RPT_WARNING, "Command function returned an error after command from client on socket %d: %.40s", c->sock, str );
 		}	
+	}
+	else {
+		snprintf (errmsg, sizeof(errmsg), "huh? Invalid command \"%.40s\"\n", argv[0]);
+		sock_send_string (c->sock, errmsg);
+		report( RPT_WARNING, "Invalid command from client on socket %d: %.40s", c->sock, str );
+	}
 		
 	free( arg_space );
 	return 0;
 }
+