[U-Boot] [RFC 2/2 v2] command: Remove maxargs from command structure

John Schmoller jschmoller at xes-inc.com
Fri Mar 12 20:25:31 CET 2010


Remove maxargs from the command structure and move bounds checking
to individual functions.

This is a subset of the changes created by the real patch, and is for
reviewing only, and not applying. The real patch is much too large for
the mailing list.

Signed-off-by: John Schmoller <jschmoller at xes-inc.com>
---
Since v1:
 - This patch is now a subset of the real patch. It is now small enough to
 be posted.

 common/cmd_eeprom.c |    7 ++---
 common/cmd_elf.c    |   18 ++++++++----
 common/cmd_i2c.c    |   75 +++++++++++++++++++--------------------------------
 common/hush.c       |   11 ++-----
 common/kgdb.c       |    2 +-
 common/lcd.c        |    5 +++-
 common/main.c       |   19 +++++--------
 include/command.h   |   22 +++++++++------
 8 files changed, 71 insertions(+), 88 deletions(-)

diff --git a/common/cmd_eeprom.c b/common/cmd_eeprom.c
index 519b510..b645f97 100644
--- a/common/cmd_eeprom.c
+++ b/common/cmd_eeprom.c
@@ -104,8 +104,7 @@ int do_eeprom ( cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 		}
 	}
 
-	cmd_usage(cmdtp);
-	return 1;
+	return CMD_ERR_USAGE;
 }
 #endif
 
@@ -422,7 +421,7 @@ void eeprom_init  (void)
 
 #ifdef CONFIG_SYS_I2C_MULTI_EEPROMS
 U_BOOT_CMD(
-	eeprom,	6,	1,	do_eeprom,
+	eeprom,	1,	do_eeprom,
 	"EEPROM sub-system",
 	"read  devaddr addr off cnt\n"
 	"eeprom write devaddr addr off cnt\n"
@@ -430,7 +429,7 @@ U_BOOT_CMD(
 );
 #else /* One EEPROM */
 U_BOOT_CMD(
-	eeprom,	5,	1,	do_eeprom,
+	eeprom,	1,	do_eeprom,
 	"EEPROM sub-system",
 	"read  addr off cnt\n"
 	"eeprom write addr off cnt\n"
diff --git a/common/cmd_elf.c b/common/cmd_elf.c
index 63f6fe7..26fa346 100644
--- a/common/cmd_elf.c
+++ b/common/cmd_elf.c
@@ -64,10 +64,13 @@ int do_bootelf (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	/* -------------------------------------------------- */
 	int rcode = 0;
 
-	if (argc < 2)
+	if (argc == 1) {
 		addr = load_addr;
-	else
+	} else if (argc == 2) {
 		addr = simple_strtoul (argv[1], NULL, 16);
+	} else {
+		return CMD_ERR_USAGE;
+	}
 
 	if (!valid_elf_image (addr))
 		return 1;
@@ -107,10 +110,13 @@ int do_bootvx (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	 * If we don't know where the image is then we're done.
 	 */
 
-	if (argc < 2)
+	if (argc == 1) {
 		addr = load_addr;
-	else
+	} else if (argc == 2) {
 		addr = simple_strtoul (argv[1], NULL, 16);
+	} else {
+		return CMD_ERR_USAGE;
+	}
 
 #if defined(CONFIG_CMD_NET)
 	/* Check to see if we need to tftp the image ourselves before starting */
@@ -311,13 +317,13 @@ unsigned long load_elf_image (unsigned long addr)
 
 /* ====================================================================== */
 U_BOOT_CMD(
-	bootelf,      2,      0,      do_bootelf,
+	bootelf,      0,      do_bootelf,
 	"Boot from an ELF image in memory",
 	" [address] - load address of ELF image."
 );
 
 U_BOOT_CMD(
-	bootvx,      2,      0,      do_bootvx,
+	bootvx,      0,      do_bootvx,
 	"Boot vxWorks from an ELF image",
 	" [address] - load address of vxWorks ELF image."
 );
diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
index 62cbd33..d4d07e5 100644
--- a/common/cmd_i2c.c
+++ b/common/cmd_i2c.c
@@ -168,10 +168,8 @@ int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	alen   = i2c_dp_last_alen;
 	length = i2c_dp_last_length;
 
-	if (argc < 3) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (argc < 3)
+		return CMD_ERR_USAGE;
 
 	if ((flag & CMD_FLAG_REPEAT) == 0) {
 		/*
@@ -193,10 +191,8 @@ int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 		for (j = 0; j < 8; j++) {
 			if (argv[2][j] == '.') {
 				alen = argv[2][j+1] - '0';
-				if (alen > 4) {
-					cmd_usage(cmdtp);
-					return 1;
-				}
+				if (alen > 4)
+					return CMD_ERR_USAGE;
 				break;
 			} else if (argv[2][j] == '\0')
 				break;
@@ -269,10 +265,8 @@ int do_i2c_mw ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	int	count;
 	int	j;
 
-	if ((argc < 4) || (argc > 5)) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if ((argc < 4) || (argc > 5))
+		return CMD_ERR_USAGE;
 
 	/*
 	 * Chip is always specified.
@@ -287,10 +281,8 @@ int do_i2c_mw ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	for (j = 0; j < 8; j++) {
 		if (argv[2][j] == '.') {
 			alen = argv[2][j+1] - '0';
-			if (alen > 4) {
-				cmd_usage(cmdtp);
-				return 1;
-			}
+			if (alen > 4)
+				return CMD_ERR_USAGE;
 			break;
 		} else if (argv[2][j] == '\0')
 			break;
@@ -343,10 +335,8 @@ int do_i2c_crc (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	ulong	err;
 	int	j;
 
-	if (argc < 4) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (argc < 4)
+		return CMD_ERR_USAGE;
 
 	/*
 	 * Chip is always specified.
@@ -361,10 +351,8 @@ int do_i2c_crc (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	for (j = 0; j < 8; j++) {
 		if (argv[2][j] == '.') {
 			alen = argv[2][j+1] - '0';
-			if (alen > 4) {
-				cmd_usage(cmdtp);
-				return 1;
-			}
+			if (alen > 4)
+				return CMD_ERR_USAGE;
 			break;
 		} else if (argv[2][j] == '\0')
 			break;
@@ -415,10 +403,8 @@ mod_i2c_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char *argv[])
 	int	j;
 	extern char console_buffer[];
 
-	if (argc != 3) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (argc != 3)
+		return CMD_ERR_USAGE;
 
 #ifdef CONFIG_BOOT_RETRY_TIME
 	reset_cmd_timeout();	/* got a good command to get here */
@@ -451,10 +437,8 @@ mod_i2c_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char *argv[])
 		for (j = 0; j < 8; j++) {
 			if (argv[2][j] == '.') {
 				alen = argv[2][j+1] - '0';
-				if (alen > 4) {
-					cmd_usage(cmdtp);
-					return 1;
-				}
+				if (alen > 4)
+					return CMD_ERR_USAGE;
 				break;
 			} else if (argv[2][j] == '\0')
 				break;
@@ -589,10 +573,8 @@ int do_i2c_loop(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	int	delay;
 	int	j;
 
-	if (argc < 3) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (argc < 3)
+		return CMD_ERR_USAGE;
 
 	/*
 	 * Chip is always specified.
@@ -607,10 +589,8 @@ int do_i2c_loop(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	for (j = 0; j < 8; j++) {
 		if (argv[2][j] == '.') {
 			alen = argv[2][j+1] - '0';
-			if (alen > 4) {
-				cmd_usage(cmdtp);
-				return 1;
-			}
+			if (alen > 4)
+				return CMD_ERR_USAGE;
 			break;
 		} else if (argv[2][j] == '\0')
 			break;
@@ -752,10 +732,9 @@ int do_sdram (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 		"32 MiB", "16 MiB", "8 MiB", "4 MiB"
 	};
 
-	if (argc < 2) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (argc < 2)
+		return CMD_ERR_USAGE;
+
 	/*
 	 * Chip is always specified.
 	 */
@@ -1244,6 +1223,9 @@ int do_i2c_bus_speed(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 
 int do_i2c(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 {
+	if (argc > 6)
+		return CMD_ERR_USAGE;
+
 	/* Strip off leading 'i2c' command argument */
 	argc--;
 	argv++;
@@ -1280,14 +1262,13 @@ int do_i2c(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 	if (!strncmp(argv[0], "sd", 2))
 		return do_sdram(cmdtp, flag, argc, argv);
 #endif
-	cmd_usage(cmdtp);
-	return 0;
+	return CMD_ERR_USAGE;
 }
 
 /***************************************************/
 
 U_BOOT_CMD(
-	i2c, 6, 1, do_i2c,
+	i2c, 1, do_i2c,
 	"I2C sub-system",
 	"speed [speed] - show or set I2C bus speed\n"
 #if defined(CONFIG_I2C_MUX)
diff --git a/common/hush.c b/common/hush.c
index 1837a7f..682846e 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -1693,12 +1693,6 @@ static int run_pipe_real(struct pipe *pi)
 					flag |= CMD_FLAG_BOOTD;
 				}
 #endif
-				/* found - check max args */
-				if (cmdtp->maxargs &&
-				    (child->argc - i) > cmdtp->maxargs) {
-					cmd_usage(cmdtp);
-					return -1;
-				}
 #endif
 				child->argv+=i;  /* XXX horrible hack */
 #ifndef __U_BOOT__
@@ -1711,7 +1705,8 @@ static int run_pipe_real(struct pipe *pi)
 				if ( !cmdtp->repeatable )
 					flag_repeat = 0;
 
-
+				if (rcode == CMD_ERR_USAGE)
+					cmd_usage(cmdtp);
 #endif
 				child->argv-=i;  /* XXX restore hack so free() can work right */
 #ifndef __U_BOOT__
@@ -3628,7 +3623,7 @@ int do_showvar (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 }
 
 U_BOOT_CMD(
-	showvar, 0, 1,	do_showvar,
+	showvar, 1,	do_showvar,
 	"print local hushshell variables",
 	"\n    - print values of all hushshell variables\n"
 	"showvar name ...\n"
diff --git a/common/kgdb.c b/common/kgdb.c
index ac47c59..0152eec 100644
--- a/common/kgdb.c
+++ b/common/kgdb.c
@@ -593,7 +593,7 @@ do_kgdb(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 }
 
 U_BOOT_CMD(
-	kgdb, 0, 1,	do_kgdb,
+	kgdb, 1,	do_kgdb,
 	"enter gdb remote debug mode",
 	"[arg0 arg1 .. argN]\n"
 	"    - executes a breakpoint so that kgdb mode is\n"
diff --git a/common/lcd.c b/common/lcd.c
index db799db..e1643b5 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -348,6 +348,9 @@ int drv_lcd_init (void)
 /*----------------------------------------------------------------------*/
 static int lcd_clear (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 {
+	if (argc > 1)
+		return CMD_ERR_USAGE;
+
 #if LCD_BPP == LCD_MONOCHROME
 	/* Setting the palette */
 	lcd_initcolregs();
@@ -392,7 +395,7 @@ static int lcd_clear (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 }
 
 U_BOOT_CMD(
-	cls,	1,	1,	lcd_clear,
+	cls,	1,	lcd_clear,
 	"clear screen",
 	""
 );
diff --git a/common/main.c b/common/main.c
index 64ec072..340478c 100644
--- a/common/main.c
+++ b/common/main.c
@@ -1396,13 +1396,6 @@ int run_command (const char *cmd, int flag)
 			continue;
 		}
 
-		/* found - check max args */
-		if (cmdtp->maxargs && argc > cmdtp->maxargs) {
-			cmd_usage(cmdtp);
-			rc = -1;
-			continue;
-		}
-
 #if defined(CONFIG_CMD_BOOTD)
 		/* avoid "bootd" recursion */
 		if (cmdtp->cmd == do_bootd) {
@@ -1420,7 +1413,11 @@ int run_command (const char *cmd, int flag)
 #endif
 
 		/* OK - call function to do the command */
-		if ((cmdtp->cmd) (cmdtp, flag, argc, argv) != 0) {
+		rc = (cmdtp->cmd) (cmdtp, flag, argc, argv);
+		if (rc == CMD_ERR_USAGE)
+			cmd_usage(cmdtp);
+
+		if (rc != 0) {
 			rc = -1;
 		}
 
@@ -1443,10 +1440,8 @@ int do_run (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 {
 	int i;
 
-	if (argc < 2) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (argc < 2)
+		return CMD_ERR_USAGE;
 
 	for (i=1; i<argc; ++i) {
 		char *arg;
diff --git a/include/command.h b/include/command.h
index 725df9f..833682e 100644
--- a/include/command.h
+++ b/include/command.h
@@ -45,7 +45,6 @@
 
 struct cmd_tbl_s {
 	char		*name;		/* Command Name			*/
-	unsigned int	maxargs;	/* maximum number of arguments	*/
 	int		repeatable;	/* autorepeat allowed?		*/
 					/* Implementation function	*/
 	int		(*cmd)(struct cmd_tbl_s *, int, int, char *[]);
@@ -105,23 +104,28 @@ extern int cmd_get_data_size(char* arg, int default_size);
 #define CMD_FLAG_REPEAT		0x0001	/* repeat last command		*/
 #define CMD_FLAG_BOOTD		0x0002	/* command is from bootd	*/
 
+/*
+ * Command Errors:
+ */
+#define CMD_ERR_USAGE		256
+
 #define Struct_Section  __attribute__ ((unused,section (".u_boot_cmd")))
 
 #ifdef  CONFIG_SYS_LONGHELP
 
-#define U_BOOT_CMD(name,maxargs,rep,cmd,usage,help) \
-cmd_tbl_t __u_boot_cmd_##name Struct_Section = {#name, maxargs, rep, cmd, usage, help}
+#define U_BOOT_CMD(name,rep,cmd,usage,help) \
+cmd_tbl_t __u_boot_cmd_##name Struct_Section = {#name, rep, cmd, usage, help}
 
-#define U_BOOT_CMD_MKENT(name,maxargs,rep,cmd,usage,help) \
-{#name, maxargs, rep, cmd, usage, help}
+#define U_BOOT_CMD_MKENT(name,rep,cmd,usage,help) \
+{#name, rep, cmd, usage, help}
 
 #else	/* no long help info */
 
-#define U_BOOT_CMD(name,maxargs,rep,cmd,usage,help) \
-cmd_tbl_t __u_boot_cmd_##name Struct_Section = {#name, maxargs, rep, cmd, usage}
+#define U_BOOT_CMD(name,rep,cmd,usage,help) \
+cmd_tbl_t __u_boot_cmd_##name Struct_Section = {#name, rep, cmd, usage}
 
-#define U_BOOT_CMD_MKENT(name,maxargs,rep,cmd,usage,help) \
-{#name, maxargs, rep, cmd, usage}
+#define U_BOOT_CMD_MKENT(name,rep,cmd,usage,help) \
+{#name, rep, cmd, usage}
 
 #endif	/* CONFIG_SYS_LONGHELP */
 
-- 
1.6.0.4



More information about the U-Boot mailing list