gbdb: refactor command parsing

Instead of passing around a string and having each command parse off
additional tokens, have a single command to parse all of the tokens,
and the commands can pass around a token list. Then each
command/subcommand can pop off whatever arguments they need.

Makefile

@@ -32,9 +32,11 @@ all: $(APPS)
 gbdb: apps/gbdb
 gbasm: apps/gbasm
 threading: apps/threading
+test-tokenize: apps/test-tokenize
+
 bmp: apps/bmp_test
 
-.PHONY: gbasm-test tests threading bmp
+.PHONY: gbasm-test tests threading bmp test-tokenize
 
 TESTS = $(patsubst %.o, %, $(filter %/test.o, $(ALL_OBJS)))
 tests: $(TESTS)

src/apps/gbdb.c

@@ -24,7 +24,7 @@
 #define INPUT_MAX_LEN 512
 #define MAX_BREAKPTS 16 /* Should be plenty for anyone */
 
-typedef void (gbdb_cmd)(char *arg_string);
+typedef void (gbdb_cmd)(struct tokens **tokens);
 
 static void breakpoint_addr_hit(uint16_t id);
 static void breakpoint_cb(struct lr35902_state *lr);
@@ -128,37 +128,35 @@ static void show_prompt()
 	fflush(stdout);
 }
 
-static void call_next_cmd(char *arg_list,
+static void call_next_cmd(struct tokens** tokens,
-						   struct tri *commands,
+						  struct tri *commands,
-						   const char *cmdname)
+						  const char *cmdname)
 {
 	char * remainder;
 	gbdb_cmd *cmd;
 	bool ambiguous = false;
-	remainder = strchr(arg_list, ' ');
+
-	if (remainder != NULL) {
+	char *token = token_next(tokens);
-		remainder[0] = '\0';
+	if (!token) {
-		remainder++;
+		return;
 	}
 
-	cmd = tri_get_string_autocomplete(commands, arg_list, &ambiguous);
+	cmd = tri_get_string_autocomplete(commands, token, &ambiguous);
-
 	if (ambiguous) {
-		gb_error("ambiguous %s command: '%s'\n", cmdname, arg_list);
+		gb_error("ambiguous %s command: '%s'\n", cmdname, token);
 		return;
 	}
-
 	if (cmd == NULL) {
-		gb_error("unrecognized %s command: '%s'\n", cmdname, arg_list);
+		gb_error("unrecognized %s command: '%s'\n", cmdname, token);
 		return;
 	}
 
-	cmd(remainder);
+	cmd(tokens);
 }
 
-static void set_logfile(char *arglist)
+static void set_logfile(struct tokens **tokens)
 {
-	char *token = token = strtok(arglist, " ");
+	char *token = token_next(tokens);
 	int fd;
 
 	if (token != NULL) {
@@ -188,17 +186,17 @@ static void set_logfile(char *arglist)
 	config.log_fd = fd;
 }
 
-static void set_quiet_on(char *arg_list)
+static void set_quiet_on(struct tokens **tokens)
 {
 	config.quiet = true;
 }
 
-static void set_quiet_off(char *arg_list)
+static void set_quiet_off(struct tokens **tokens)
 {
 	config.quiet = false;
 }
 
-static void set_quiet(char *arg_list)
+static void set_quiet(struct tokens **tokens)
 {
 	static bool init = false;
 	static struct tri commands;
@@ -212,15 +210,15 @@ static void set_quiet(char *arg_list)
 		init = true;
 	}
 
-	call_next_cmd(arg_list, &commands, "set quiet");
+	call_next_cmd(tokens, &commands, "set quiet");
 }
 
-static void set_video_debug(char *arg_list)
+static void set_video_debug(struct tokens **tokens)
 {
 	video.debug_logging = true;
 }
 
-static void set(char *arg_list)
+static void set(struct tokens **tokens)
 {
 	static bool init = false;
 	static struct tri commands;
@@ -233,16 +231,17 @@ static void set(char *arg_list)
 		init = true;
 	}
 
-	call_next_cmd(arg_list, &commands, "set");
+	call_next_cmd(tokens, &commands, "set");
 }
 
-static void echo(char *arg_list)
+static void echo(struct tokens **tokens)
 {
-	if (arg_list != NULL) {
+	char *token;
-		gb_log("%s\n", arg_list);
+	while ((token = token_next(tokens)) != NULL) {
-	} else {
+		gb_log("%s ", token);
-		gb_log("\n");
 	}
+
+	gb_log("\n");
 }
 
 static void breakpoint_cb(struct lr35902_state *lr)
@@ -256,11 +255,11 @@ static void breakpoint_cb(struct lr35902_state *lr)
 }
 
 
-static void step(char *arg_list)
+static void step(struct tokens **tokens)
 {
 	uint64_t steps, end_steps;
 	char *token;
-	token = strtok(arg_list, " ");
+	token = token_next(tokens);
 	if (token == NULL) {
 		steps = 1;
 	} else {
@@ -293,7 +292,7 @@ static void step(char *arg_list)
 	paused_breakpoint = 0;
 }
 
-static void regs(char *arg_list)
+static void regs(struct tokens **tokens)
 {
 	uint8_t f = (cpu.nf << CPU_F_BIT_POS_N) | (cpu.zf << CPU_F_BIT_POS_Z) |
 		(cpu.cf << CPU_F_BIT_POS_C) | (cpu.hf << CPU_F_BIT_POS_H);
@@ -306,21 +305,21 @@ static void regs(char *arg_list)
 	gb_log("SP: 0x%04x\n", cpu.sp);
 }
 
-static void peek(char *arg_list)
+static void peek(struct tokens **tokens)
 {
 	uint16_t pc = lr35902_get_reg_16(&cpu, LR35902_REG_PC);
 	uint8_t byte = gb_mem_read(memory, pc);
 	gb_log("0x%04x:%s\n", pc, gb_byte_to_opcode(byte));
 }
 
-static void vstep(char *arg_list)
+static void vstep(struct tokens **tokens)
 {
 	peek(NULL);
-	step(arg_list);
+	step(tokens);
 	regs(NULL);
 }
 
-static void stats(char *arg_list)
+static void stats(struct tokens **tokens)
 {
 	gb_log("Cycles: %" PRId64 "\n", cpu.metrics.cycles);
 	gb_log("Retired Insructions %" PRId64 "\n", cpu.metrics.retired_instrs);
@@ -328,7 +327,13 @@ static void stats(char *arg_list)
 	gb_log("Memory Writes: %" PRId64 "\n", cpu.metrics.mem_writes);
 }
 
-static void help(char *arg_list)
+static void comment(struct tokens **tokens)
+{
+	/* Gobble up everything */
+	do {} while (token_next(tokens));
+}
+
+static void help(struct tokens **tokens)
 {
 	gb_log(usage);
 }
@@ -376,26 +381,26 @@ static int64_t parse_val(const char *str)
 	return strtoll(str, NULL, 0);
 }
 
-static void assert(char *arg_list)
+static void assert(struct tokens **tokens)
 {
-	char *val0_str, *val1_str, *operator_str;
+	char *val0_str = NULL, *val1_str = NULL, *operator_str = NULL;
 	const char *usage = "usage: assert <value> <operator> <value>\n";
 	int64_t val0, val1;
 
-	val0_str = strtok(arg_list, " ");
+	val0_str = token_next(tokens);
 	if (val0_str == NULL) {
 		gb_error("%s", usage);
 		return;
 	}
 	val0 = parse_val(val0_str);
 
-	operator_str = strtok(NULL, " ");
+	operator_str = strdup(token_next(tokens));
 	if (operator_str == NULL) {
 		gb_error("%s", usage);
 		return;
 	}
 
-	val1_str = strtok(NULL, " ");
+	val1_str = token_next(tokens);
 	if (val1_str == NULL) {
 		gb_error("%s", usage);
 		return;
@@ -434,15 +439,20 @@ static void assert(char *arg_list)
 fail:
 	gb_error("ASSERT: %s %s %s\n", val0_str, operator_str, val1_str);
 	gb_error("%s=%" PRId64 ", %s=%" PRId64 "\n", val0_str, val0, val1_str, val1);
+	free(val0_str);
+	free(val1_str);
+	free(operator_str);
 	regs(NULL);
 	stats(NULL);
 	exit(1);
 }
 
-static void mem(char *arg_list)
+static void mem(struct tokens **tokens)
 {
 	uint16_t addr, bytes, i;
-	char *token = strtok(arg_list, " ");
+	char *token = token_next(tokens);
+
+	gb_log("mem_addr=%s\n", token);
 
 	if (token == NULL) {
 		gb_error("usage: mem <addr> (num bytes) (format)\n");
@@ -450,14 +460,16 @@ static void mem(char *arg_list)
 	}
 
 	addr = parse_val(token);
-	token = strtok(NULL, " ");
+	token = token_next(tokens);
+	gb_log("mem_bytes=%s\n", token);
 	if (token == NULL) {
 		bytes = 1;
 	} else {
 		bytes = parse_val(token);
 	}
 
-	token = strtok(NULL, " ");
+	token = token_next(tokens);
+	gb_log("mem_format=%s\n", token);
 
 	for (i = 0; i < bytes; i++) {
 		//TODO: Make sure this has no side effects
@@ -474,34 +486,32 @@ static void mem(char *arg_list)
 
 }
 
-static void load(char *arg_list)
+static void load(struct tokens **tokens)
 {
-	char *token = strtok(arg_list, " ");
+	char *token = token_next(tokens);
 	if (token == NULL) {
 		gb_error("usage: load <file>\n");
 		return;
 	}
 
-	strip_newline(token);
+	gb_log("loading %s\n", token);
 
 	gb_memory_load_file(memory, GB_MEMORY_CART, token);
 }
 
-static void load_bootrom(char *arg_list)
+static void load_bootrom(struct tokens **tokens)
 {
-	char *token = strtok(arg_list, " ");
+	char *token = token_next(tokens);
 	if (token == NULL) {
 		gb_error("usage: bootrom <file>\n");
 		return;
 	}
 
-	strip_newline(token);
-
 	gb_memory_load_file(memory, GB_MEMORY_BOOTROM, token);
 }
 
 
-static void quit(char *arg_list)
+static void quit(struct tokens **tokens)
 {
 	exit(0);
 }
@@ -528,7 +538,7 @@ static __attribute__((hot)) void do_run(void) {
 	paused_signal = 0;
 }
 
-static void run(char *arg_list)
+static void run(struct tokens **tokens )
 {
 	do_run();
 }
@@ -581,10 +591,10 @@ static int do_set_breakpoint(uint16_t addr, bool temp)
 	}
 }
 
-static void set_breakpoint(char *arg_string)
+static void set_breakpoint(struct tokens **tokens)
 {
 	uint16_t addr, i;
-	char *token = strtok(arg_string, " ");
+	char *token = token_next(tokens);
 
 	if (token == NULL) {
 		gb_error("usage: breakpoint add <addr>\n");
@@ -595,10 +605,10 @@ static void set_breakpoint(char *arg_string)
 	do_set_breakpoint(addr, false);
 }
 
-static void runto(char *arg_string)
+static void runto(struct tokens **tokens)
 {
 	uint16_t addr, i, rc;
-	char *token = strtok(arg_string, " ");
+	char *token = token_next(tokens);
 
 	if (token == NULL) {
 		gb_error("usage: runto <addr>\n");
@@ -657,9 +667,9 @@ static void breakpoint_addr_hit(uint16_t addr)
 	}
 }
 
-static void delete_breakpoint(char *arg_string)
+static void delete_breakpoint(struct tokens **tokens)
 {
-	char *token = strtok(arg_string, " ");
+	char *token = token_next(tokens);
 	int rc, bkpt;
 
 	if (token == NULL) {
@@ -675,7 +685,7 @@ static void delete_breakpoint(char *arg_string)
 	}
 }
 
-static void display_breakpoints(char *arg_string)
+static void display_breakpoints(struct tokens **tokens)
 {
 	int i;
 
@@ -688,7 +698,7 @@ static void display_breakpoints(char *arg_string)
 	}
 }
 
-static void breakpoint(char *arg_list)
+static void breakpoint(struct tokens **tokens)
 {
 	static bool init = false;
 	static struct tri commands;
@@ -706,7 +716,7 @@ static void breakpoint(char *arg_list)
 		init = true;
 	}
 
-	call_next_cmd(arg_list, &commands, "breakpoint");
+	call_next_cmd(tokens, &commands, "breakpoint");
 }
 
 static bool breakpoint_is_at_addr(uint16_t addr) {
@@ -723,70 +733,24 @@ static bool breakpoint_is_at_addr(uint16_t addr) {
 
 static void process_cmd(const char *cmd_str, struct tri *commands)
 {
-	char cmd_buffer[INPUT_MAX_LEN]; /* Buffer for command parsing */
+	struct tokens *statements;
-	bool ambiguous;
+	struct tokens *tokens;
-	gbdb_cmd *cmd;
+	char *statement, *token;
 
-	/* Current point in the line (there may be more than
+	statements = tokenize(cmd_str, ";\n");
-	 * one command on a line) */
+	if (!statements) {
-	const char *process_str;
+		return;
-
-	/* String containing the arguments following a command */
-	char *arg_str;
-
-	process_str = cmd_str;
-
-	char *comment;
-	comment = strchr(process_str, '#');
-	if (comment != NULL) {
-		*comment = '\0';
 	}
 
-	do {
+	while ((statement = token_next(&statements)) != NULL) {
-		int i;
+		tokens = tokenize(statement, " ");
-
+		if (!tokens) {
-		while (process_str[0] == ' ') { process_str++; };
+			continue;
-		for (i = 0;; i++) {
-			if (process_str[i] == '\0' ||
-			    process_str[i] == ';') {
-				cmd_buffer[i] = '\0';
-				break;
-			} else {
-				cmd_buffer[i] = process_str[i];
-			}
 		}
 
-		strip_newline(cmd_buffer);
+		call_next_cmd(&tokens, commands, "");
-		arg_str = strchr(cmd_buffer, ' ');
+	}
-		if (arg_str != NULL) {
-			arg_str[0] = '\0';
-			arg_str++;
-		}
 
-		if (strlen(cmd_buffer) == 0) {
-			return;
-		}
-
-
-		cmd = tri_get_string_autocomplete(commands, cmd_buffer, &ambiguous);
-
-		if (ambiguous) {
-			gb_error("ambiguous command: '%s'\n", cmd_buffer);
-			break;
-		} else if (cmd == NULL) {
-			gb_error("unrecognized command: '%s'\n", cmd_buffer);
-			break;
-		} else {
-			cmd(arg_str);
-		}
-
-		process_str = strchr(process_str, ';');
-		if (process_str != NULL) {
-			process_str++;
-		} else {
-			break;
-		}
-	} while (1);
 }
 
 static int load_initfile(const char *initfile_path, struct tri *commands)
@@ -818,6 +782,7 @@ int main(int argc, char **argv)
 	old_buffer[0] = '\0';
 
 	tri_init(&commands);
+	tri_add_string(&commands, "#", comment);
 	tri_add_string(&commands, "step", step);
 	tri_add_string(&commands, "vstep", vstep);
 	tri_add_string(&commands, "run", run);

src/common/common.c

@@ -45,9 +45,11 @@ struct token {
 
 struct tokens* tokenize(const char *string, const char *delims)
 {
-	struct tokens *tokens = malloc(sizeof(*tokens));
+	struct tokens *tokens = malloc(sizeof(struct tokens));
-	struct token *token = malloc(sizeof(*token));
+	struct token *token = malloc(sizeof(struct token));
-	char *prev, *delim;
+	struct token *prev_token = NULL;
+	const char *prev = string;
+	const char *delim;
 	int done = 0;
 
 	tokens->next = token;
@@ -61,37 +63,44 @@ struct tokens* tokenize(const char *string, const char *delims)
 		if (delim == NULL) {
 			has_token = strlen(prev) > 0;
 			done = 1;
-			break;
+		} else if (delim == prev) {
-		} else if (delim - string == 1) {
 			/* Adjacent delimiters, skip them */
-			prev = delim;
+			prev++;
 			continue;
 		} else {
 			has_token = 1;
 		}
 
 		if (has_token) {
+			prev_token = token;
 			token->next = malloc(sizeof(struct tokens));
 			token->string = strndup(prev, delim - prev);
 			token = token->next;
 		}
 
+		prev = delim + 1;
 	}
 
-	if (tokens->next->string == NULL) {
+	if (prev_token == NULL) {
-		free(tokens->next);
+		free(token);
 		free(tokens);
 		return NULL;
 	} else {
-		free(token);
+		free(prev_token->next);
+		prev_token->next = NULL;
 		return tokens;
 	}
 }
 
-char *token_next(struct tokens* tokens)
+char *token_next(struct tokens **tokens_p)
 {
+	struct tokens *tokens = *tokens_p;
 	struct token *prev;
 
+	if (!tokens) {
+		return NULL;
+	}
+
 	if (tokens->next) {
 		if (tokens->old) {
 			free(tokens->old);
@@ -106,6 +115,7 @@ char *token_next(struct tokens* tokens)
 	} else {
 		free(tokens->old);
 		free(tokens);
+		*tokens_p = NULL;
 		return NULL;
 	}
 }

src/common/common.h

@@ -19,7 +19,7 @@ void downcase(char *str);
 struct tokens;
 
 struct tokens* tokenize(const char *string, const char *delims);
-char *token_next(struct tokens* tokens);
+char *token_next(struct tokens **tokens);
 
 #ifdef DEBUG
 #define DEBUG_ON 1