Implemented shell argument validation; implemented fancy help printout

shell.c

@@ -88,12 +88,63 @@ static int shell_run_function(shell_context_t *ctx, const shell_command_t *cmd,
 	shell_run_data_t *data = arg;
 
 	if(strcmp(cmd->cmd, data->argv[0]) == 0) {
+		int invalid = 0;
+
+		if(cmd->args == NULL && data->argc != 1)
+			invalid = 1;
+		else if(cmd->args) {
+			char *dup = strdup(cmd->args), *save = dup, *ptrptr = NULL;
+
+			int pos = 1;
+			int max_tokens = 1;
+
+			for(char *token = strtok_r(dup, " ", &ptrptr); token; token = strtok_r(NULL, " ", &ptrptr)) {
+				if(strcmp(token, "...") == 0) {
+					max_tokens = -1;
+
+					break;
+				}
+
+				max_tokens++;
+
+				if(data->argc - 1 < pos) {
+					if(*token == '[') {
+						break;
+					} else if(*token == '<') {
+						invalid = 1;
+
+						break;
+					}
+				}
+
+				pos++;
+			}
+
+			if(max_tokens != -1 && data->argc > max_tokens)
+				invalid = 1;
+
+
+			free(save);
+		}
+
+		if(invalid) {
+			if(cmd->args)
+				fprintf(stderr, "Usage: %s %s\n", cmd->cmd, cmd->args);
+			else
+				fprintf(stderr, "Usage: %s\n", cmd->cmd);
+
+			errno = EINVAL;
+
+			return -1;
+
+		} else {
 			int ret = cmd->handler(ctx, data->argc, data->argv);
 
 			if(ret == 0)
 				return 1;
 			else
 				return ret;
+		}
 	} else
 		return 0;
 }

shell.h

@@ -27,6 +27,7 @@ typedef struct shell_command {
 	const char *cmd;
 	const char *description;
 	int (*handler)(shell_context_t *ctx, int argc, char *argv[]);
+	const char *args;
 } shell_command_t;
 
 shell_context_t *shell_init(void *ingenic);

shell_builtins.c

@@ -38,33 +38,62 @@ static int builtin_set(shell_context_t *ctx, int argc, char *argv[]);
 static int builtin_safe(shell_context_t *ctx, int argc, char *argv[]);
 
 const shell_command_t builtin_cmdset[] = {
-	{ "help", "- Display this message", builtin_help },
+	{ "help", "Display this message", builtin_help, NULL },
-	{ "exit", "- Batch: stop current script, interactive: end session", builtin_exit },
+	{ "exit", "Batch: stop current script, interactive: end session", builtin_exit, NULL },
-	{ "source", "<FILENAME> - run specified script", builtin_source },
+	{ "source", "Run specified script", builtin_source, "<FILENAME>" },
-	{ "echo", "<STRING> - output specified string", builtin_echo },
+	{ "echo", "Output specified string", builtin_echo,  "<STRING> ..." },
-	{ "sleep", "<MILLISECONDS> - sleep a specified amount of time", builtin_sleep },
+	{ "sleep", "Sleep a specified amount of time", builtin_sleep, "<MILLISECONDS>" },
-	{ "set", "[VARIABLE] [VALUE] - print or set configuraton variables", builtin_set },
+	{ "set", "Print or set configuraton variables", builtin_set, "[VARIABLE] [VALUE]" },
-	{ "safe", "<COMMAND> [ARG]... - run command ignoring errors", builtin_safe },
+	{ "safe", "Run command ignoring errors", builtin_safe, "<COMMAND> [ARG] ..." },
 
-	{ "redetect", " - Redetect CPU", builtin_redetect },
+	{ "redetect", "Redetect CPU", builtin_redetect, NULL },
-	{ "rebuildcfg", " - Rebuild firmware configuration data", builtin_rebuildcfg },
+	{ "rebuildcfg", "Rebuild firmware configuration data", builtin_rebuildcfg, NULL },
 
 	{ NULL, NULL, NULL }
 };
 
-static int builtin_help(shell_context_t *ctx, int argc, char *argv[]) {
+static int help_maxwidth_function(shell_context_t *ctx, const shell_command_t *cmd, void *arg) {
-/*	for(int i = 0; commands[i].cmd != NULL; i++) {
+	int len = strlen(cmd->cmd), *maxlen = arg;
-		printf("%s %s\n", commands[i].cmd, commands[i].description);
-	}
 
-	if(set_cmds) {
+	if(cmd->args)
-		for(int i = 0; set_cmds[i].cmd != NULL; i++)
+		len += strlen(cmd->args) + 1;
-			printf("%s %s\n", set_cmds[i].cmd, set_cmds[i].description);
+
-	}*/
+	if(len > *maxlen)
+		*maxlen = len;		
 
 	return 0;
 }
 
+static int help_print_function(shell_context_t *ctx, const shell_command_t *cmd, void *arg) {
+	int len = strlen(cmd->cmd), *maxlen = arg;
+
+	fputs(cmd->cmd, stdout);
+
+	if(cmd->args) {
+		len += strlen(cmd->args) + 1;
+
+		putchar(' ');
+		fputs(cmd->args, stdout);
+	}
+
+	for(int i = 0; i < *maxlen - len; i++)
+		putchar(' ');
+
+	puts(cmd->description);
+
+	return 0;
+}
+
+static int builtin_help(shell_context_t *ctx, int argc, char *argv[]) {
+	int maxwidth = 0;
+
+	shell_enumerate_commands(ctx, help_maxwidth_function, &maxwidth);
+
+	maxwidth += 2;
+
+	return shell_enumerate_commands(ctx, help_print_function, &maxwidth);
+}
+
 static int builtin_exit(shell_context_t *ctx, int argc, char *argv[]) {
 	shell_exit(ctx, 1);
 
@@ -72,12 +101,6 @@ static int builtin_exit(shell_context_t *ctx, int argc, char *argv[]) {
 }
 
 static int builtin_source(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc != 2) {
-		printf("Usage: %s <FILENAME>\n", argv[0]);
-
-		return -1;
-	}
-
 	int ret = shell_source(ctx, argv[1]);
 
 	if(ret == -1) {
@@ -90,12 +113,6 @@ static int builtin_source(shell_context_t *ctx, int argc, char *argv[]) {
 }
 
 static int builtin_echo(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc < 2) {
-		printf("Usage: %s <STRING>\n", argv[0]);
-
-		return -1;
-	}
-
 	for(int i = 1; i < argc; i++) {
 		fputs(argv[i], stdout);
 
@@ -106,12 +123,6 @@ static int builtin_echo(shell_context_t *ctx, int argc, char *argv[]) {
 }
 
 static int builtin_sleep(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc != 2) {
-		printf("Usage: %s <MILLISECONDS>\n", argv[0]);
-
-		return -1;
-	}
-
 	uint32_t ms = atoi(argv[1]);
 
 	usleep(ms * 1000);
@@ -120,12 +131,6 @@ static int builtin_sleep(shell_context_t *ctx, int argc, char *argv[]) {
 }
 
 static int builtin_redetect(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc != 1) {
-		printf("Usage: %s\n", argv[0]);
-
-		return -1;
-	}
-
 	if(ingenic_redetect(shell_device(ctx)) == -1) {
 		perror("ingenic_redetect");
 
@@ -147,11 +152,6 @@ static int builtin_set(shell_context_t *ctx, int argc, char *argv[]) {
 
 	} else if(argc == 3) {
 		cfg_setenv(argv[1], argv[2]);
-
-	} else {
-		printf("Usage: %s [VARIABLE] [VALUE]\n", argv[0]);
-
-		return -1;
 	}
 
 	return 0;
@@ -159,22 +159,10 @@ static int builtin_set(shell_context_t *ctx, int argc, char *argv[]) {
 
 
 static int builtin_rebuildcfg(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc != 1) {
-		printf("Usage: %s\n", argv[0]);
-
-		return -1;
-	}
-
 	return ingenic_rebuild(shell_device(ctx));
 }
 
 static int builtin_safe(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc < 2) {
-		printf("Usage: %s <COMMAND> [ARG]...\n", argv[0]);
-
-		return -1;
-	}
-
 	if(shell_run(ctx, argc - 1, argv + 1) == -1)
 		perror("shell_run");
 

spl_cmdset.c

@@ -31,10 +31,10 @@ static int spl_gpio(shell_context_t *ctx, int argc, char *argv[]);
 static int spl_boot(shell_context_t *ctx, int argc, char *argv[]);
 
 const shell_command_t spl_cmdset[] = {
-	{ "memtest", "[BASE <SIZE>] - SDRAM test", spl_memtest },
+	{ "memtest", "SDRAM test", spl_memtest, "[BASE] <SIZE>" },
-	{ "gpio", "<PIN> <STATE> - Set GPIO #PIN to STATE 0 or 1", spl_gpio },
+	{ "gpio", "Set GPIO #PIN to STATE 0 or 1", spl_gpio, "<PIN> <STATE>" },
-	{ "boot", " - Load stage2 USB bootloader", spl_boot },
+	{ "boot", "Load stage2 USB bootloader", spl_boot, NULL },
-	{ NULL, NULL, NULL }
+	{ NULL, NULL, NULL, NULL }
 };
 
 static int spl_stage1_op(shell_context_t *ctx, uint32_t op, uint32_t pin, uint32_t base, uint32_t size) {
@@ -53,12 +53,6 @@ static int spl_stage1_op(shell_context_t *ctx, uint32_t op, uint32_t pin, uint32
 }
 
 static int spl_memtest(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc != 1 && argc != 3) {
-		printf("Usage: %s [BASE <SIZE>]\n", argv[0]);
-
-		return -1;
-	}
-
 	uint32_t start, size;
 
 	if(argc == 3) {
@@ -94,7 +88,7 @@ static int spl_memtest(shell_context_t *ctx, int argc, char *argv[]) {
 }
 
 static int spl_gpio(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc != 3 || (strcmp(argv[2], "0") && strcmp(argv[2], "1"))) {
+	if(strcmp(argv[2], "0") && strcmp(argv[2], "1")) {
 		printf("Usage: %s <PIN> <STATE>\n", argv[0]);
 		printf("  STATE := 0 | 1\n");
 
@@ -105,10 +99,6 @@ static int spl_gpio(shell_context_t *ctx, int argc, char *argv[]) {
 }
 
 static int spl_boot(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc != 1) {
-		printf("Usage: %s\n", argv[0]);
-	}
-
 	int ret = spl_stage1_op(ctx, STAGE1_DEBUG_BOOT, 0, 0, 0);
 
 	if(ret == -1)

usbboot_cmdset.c

@@ -36,19 +36,19 @@ static int usbboot_nload(shell_context_t *ctx, int argc, char *argv[]);
 
 const shell_command_t usbboot_cmdset[] = {
 
-	{ "boot", "- Reconfigure stage2", usbboot_boot },
+	{ "boot", "Reconfigure stage2", usbboot_boot, NULL },
-	{ "load", "<FILE> <BASE> - Load file to SDRAM", usbboot_load },
+	{ "load", "Load file to SDRAM", usbboot_load, "<FILE> <BASE>" },
-	{ "go", "<ADDRESS> - Jump to <ADDRESS>", usbboot_go },
+	{ "go", "Jump to <ADDRESS>", usbboot_go, "<ADDRESS>" },
 
-	{ "nquery", "<DEVICE> - Query NAND information", usbboot_nquery },
+	{ "nquery", "Query NAND information", usbboot_nquery, "<DEVICE>" },
-	{ "ndump", "<DEVICE> <STARTPAGE> <PAGES> <FILE> - Dump NAND to file", usbboot_ndump },
+	{ "ndump", "Dump NAND to file", usbboot_ndump, "<DEVICE> <STARTPAGE> <PAGES> <FILE>" },
-	{ "ndump_oob", "<DEVICE> <STARTPAGE> <PAGES> <FILE> - Dump NAND with OOB to file", usbboot_ndump },
+	{ "ndump_oob", "Dump NAND with OOB to file", usbboot_ndump, "<DEVICE> <STARTPAGE> <PAGES> <FILE>" },
-	{ "nerase", "<DEVICE> <STARTBLOCK> <BLOCKS> - Erase NAND blocks", usbboot_nerase },
+	{ "nerase", "Erase NAND blocks", usbboot_nerase, "<DEVICE> <STARTBLOCK> <BLOCKS>" },
-	{ "nprogram", "<DEVICE> <STARTPAGE> <FILE> - Program NAND from file", usbboot_nprogram },
+	{ "nprogram", "Program NAND from file", usbboot_nprogram, "<DEVICE> <STARTPAGE> <FILE>" },
-	{ "nprogram_oob", "<DEVICE> <STARTPAGE> <FILE> - Program NAND with OOB from file", usbboot_nprogram },
+	{ "nprogram_oob", "Program NAND with OOB from file", usbboot_nprogram, "<DEVICE> <STARTPAGE> <FILE>" },
-	{ "nload", "<DEVICE> <STARTPAGE> <PAGES> <BASE> - Load NAND data to SDRAM", usbboot_nload },
+	{ "nload", "Load NAND data to SDRAM", usbboot_nload, "<DEVICE> <STARTPAGE> <PAGES> <BASE>" },
 	
-	{ NULL, NULL, NULL }
+	{ NULL, NULL, NULL, NULL }
 };
 
 static int usbboot_boot(shell_context_t *ctx, int argc, char *argv[]) {
@@ -61,12 +61,6 @@ static int usbboot_boot(shell_context_t *ctx, int argc, char *argv[]) {
 }
 
 static int usbboot_load(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc != 3) {
-		printf("Usage: %s <FILE> <BASE>\n", argv[0]);
-
-		return -1;
-	}
-
 	int ret = ingenic_load_sdram_file(shell_device(ctx), strtoul(argv[2], NULL, 0), argv[1]);
 
 	if(ret == -1)
@@ -76,12 +70,6 @@ static int usbboot_load(shell_context_t *ctx, int argc, char *argv[]) {
 }
 
 static int usbboot_go(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc != 2) {
-		printf("Usage: %s <ADDRESS>\n", argv[0]);
-
-		return -1;
-	}
-
 	int ret = ingenic_go(shell_device(ctx), strtoul(argv[1], NULL, 0));
 
 	if(ret == -1)
@@ -91,12 +79,6 @@ static int usbboot_go(shell_context_t *ctx, int argc, char *argv[]) {
 }
 
 static int usbboot_nquery(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc != 2) {
-		printf("Usage: %s <DEVICE>\n", argv[0]);
-
-		return -1;
-	}
-
 	nand_info_t info;
 
 	int ret = ingenic_query_nand(shell_device(ctx), atoi(argv[1]), &info);
@@ -120,12 +102,6 @@ static int usbboot_nquery(shell_context_t *ctx, int argc, char *argv[]) {
 }
 
 static int usbboot_ndump(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc != 5) {
-		printf("Usage: %s <DEVICE> <STARTPAGE> <PAGES> <FILE>\n", argv[0]);
-		
-		return -1;
-	}
-	
 	int type = strcmp(argv[0], "ndump_oob") ? NO_OOB : OOB_ECC;
 	
 	if(cfg_getenv("NAND_RAW"))
@@ -140,12 +116,6 @@ static int usbboot_ndump(shell_context_t *ctx, int argc, char *argv[]) {
 }
 
 static int usbboot_nerase(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc != 4) {
-		printf("Usage: %s <DEVICE> <STARTBLOCK> <BLOCKS>\n", argv[0]);
-		
-		return -1;
-	}
-	
 	int ret = ingenic_erase_nand(shell_device(ctx), atoi(argv[1]), atoi(argv[2]), atoi(argv[3]));
 	
 	if(ret == -1)
@@ -156,12 +126,6 @@ static int usbboot_nerase(shell_context_t *ctx, int argc, char *argv[]) {
 }
 
 static int usbboot_nprogram(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc != 4) {
-		printf("Usage: %s <DEVICE> <STARTPAGE> <FILE>\n", argv[0]);
-		
-		return -1;
-	}
-	
 	int type = strcmp(argv[0], "nprogram_oob") ? NO_OOB : OOB_ECC;
 	
 	if(strcmp(argv[0], "nprogram_oob") == 0) {
@@ -181,12 +145,6 @@ static int usbboot_nprogram(shell_context_t *ctx, int argc, char *argv[]) {
 }
 
 static int usbboot_nload(shell_context_t *ctx, int argc, char *argv[]) {
-	if(argc != 5) {
-		printf("Usage: %s <DEVICE> <STARTPAGE> <PAGES> <BASE>\n", argv[0]);
-		
-		return -1;
-	}
-	
 	int ret = ingenic_load_nand(shell_device(ctx), atoi(argv[1]), atoi(argv[2]), atoi(argv[3]), strtoul(argv[4], NULL, 0));
 	
 	if(ret == -1)