All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [RFC 0/2] Remove CONFIG_SYS_MAXARGS
@ 2010-03-12 15:51 John Schmoller
  2010-03-12 15:51 ` [U-Boot] [RFC 1/2] cmd: " John Schmoller
  2010-03-12 17:12 ` [U-Boot] [RFC 0/2] " Wolfgang Denk
  0 siblings, 2 replies; 3+ messages in thread
From: John Schmoller @ 2010-03-12 15:51 UTC (permalink / raw)
  To: u-boot

The first patch removes CONFIG_SYS_MAXARGS, replacing the staticly defined
array with a malloc'd array of the appropriate size. When a function has no
upper argument limit (ie, was set to CONFIG_SYS_MAXARGS), it is now set to 0
to indicate this fact.  Argument count is now unlimited, within reason and
malloc buffer size.

The second patch removes cmdtp->maxargs and moves the checks to the individual
command functions.  Since most functions do bounds checking anyway, it's a
a fairly cheap task (sometimes free) to remove this bounds check in
common/main.c.  In addition, it's more intuitive (in my opinion) if all bounds
checking is done in only one place for each function.  The second patch also
creates a CMD_ERR_USAGE return value, which prints usage when returned.  The
overall effect of this patch is to reduce code size by an average of 200-250
bytes and, I feel, make things a bit cleaner.

I'm looking for comments on these two patches as they are quite invasive, and
will definitly cause problems for those people who maintain their own code
out-of-tree.  They may also require an additional amount of testing.

John Schmoller (2):
  cmd: Remove CONFIG_SYS_MAXARGS
  command: Remove maxargs from command structure

 board/BuS/EB+MCF-EV123/EB+MCF-EV123.c         |    8 +-
 board/BuS/eb_cpux9k2/cpux9k2.c                |    5 +-
 board/MAI/AmigaOneG3SE/cmd_boota.c            |    6 +-
 board/MAI/menu/cmd_menu.c                     |    6 +-
 board/amcc/acadia/cmd_acadia.c                |    8 +-
 board/amcc/luan/luan.c                        |    5 +-
 board/amcc/makalu/cmd_pll.c                   |   11 +--
 board/amcc/taihu/lcd.c                        |   31 +++---
 board/amcc/taihu/taihu.c                      |   22 ++---
 board/amcc/taihu/update.c                     |    5 +-
 board/amcc/taishan/lcd.c                      |   48 ++++++----
 board/amcc/taishan/showinfo.c                 |   15 +++-
 board/amcc/taishan/update.c                   |    5 +-
 board/amcc/yucca/cmd_yucca.c                  |    8 +-
 board/amirix/ap1000/ap1000.c                  |   21 +++-
 board/amirix/ap1000/powerspan.c               |    4 +-
 board/barco/barco.c                           |   15 +--
 board/bc3450/cmd_bc3450.c                     |   44 +++++----
 board/bf537-stamp/cmd_bf537led.c              |    2 +-
 board/cm-bf527/gpio.c                         |    5 +-
 board/cm-bf537e/flash.c                       |    5 +-
 board/cm-bf537u/flash.c                       |    5 +-
 board/cm5200/cmd_cm5200.c                     |    2 +-
 board/delta/delta.c                           |    5 +-
 board/digsy_mtc/cmd_mtc.c                     |   48 ++++-----
 board/esd/ar405/ar405.c                       |   20 +++--
 board/esd/cms700/cms700.c                     |    5 +-
 board/esd/common/auto_update.c                |    5 +-
 board/esd/common/cmd_loadpci.c                |    5 +-
 board/esd/common/lcd.c                        |    8 +-
 board/esd/common/xilinx_jtag/micro.c          |    8 +-
 board/esd/cpci2dp/cpci2dp.c                   |    5 +-
 board/esd/cpci405/cpci405.c                   |   14 ++-
 board/esd/cpci5200/cpci5200.c                 |    5 +-
 board/esd/cpci750/cpci750.c                   |   10 ++-
 board/esd/dasa_sim/cmd_dasa_sim.c             |    8 +-
 board/esd/du440/du440.c                       |   34 +++++--
 board/esd/hh405/hh405.c                       |    5 +-
 board/esd/ocrtc/cmd_ocrtc.c                   |   10 ++-
 board/esd/pci405/cmd_pci405.c                 |    5 +-
 board/esd/pci405/pci405.c                     |    8 +-
 board/esd/pf5200/pf5200.c                     |   15 +++-
 board/esd/plu405/plu405.c                     |    5 +-
 board/esd/pmc405de/pmc405de.c                 |   21 +++-
 board/esd/pmc440/cmd_pmc440.c                 |   58 +++++++-----
 board/esd/tasreg/tasreg.c                     |   45 ++++-----
 board/esd/vme8349/caddy.c                     |    8 +-
 board/esd/voh405/voh405.c                     |    5 +-
 board/evb64260/zuma_pbb.c                     |   15 +++-
 board/fads/fads.h                             |    1 -
 board/freescale/common/pixis.c                |    9 +-
 board/freescale/common/sys_eeprom.c           |    9 +-
 board/freescale/mpc8610hpcd/mpc8610hpcd_diu.c |    8 +-
 board/g2000/g2000.c                           |   15 +++-
 board/hymod/bsp.c                             |   10 +-
 board/inka4x0/inkadiag.c                      |   31 +++---
 board/keymile/common/keymile_hdlc_enet.c      |   10 ++-
 board/keymile/km_arm/km_arm.c                 |   11 +--
 board/lwmon/lwmon.c                           |   15 ++--
 board/lwmon5/kbd.c                            |    5 +-
 board/lwmon5/lwmon5.c                         |   11 +--
 board/micronas/vct/smc_eeprom.c               |   15 +++-
 board/mpl/common/common_util.c                |    3 +-
 board/mpl/mip405/cmd_mip405.c                 |    5 +-
 board/mpl/pati/cmd_pati.c                     |    5 +-
 board/mpl/pip405/cmd_pip405.c                 |    5 +-
 board/mpl/vcma9/cmd_vcma9.c                   |    6 +-
 board/pcippc2/pcippc2.c                       |    5 +-
 board/pcs440ep/pcs440ep.c                     |   12 ++-
 board/pn62/cmd_pn62.c                         |   14 +--
 board/prodrive/pdnb3/pdnb3.c                  |    8 +-
 board/pxa255_idp/pxa_idp.c                    |    2 +-
 board/r360mpi/r360mpi.c                       |    5 +-
 board/renesas/sh7785lcr/rtl8169_mac.c         |   16 +--
 board/renesas/sh7785lcr/selfcheck.c           |   11 +--
 board/renesas/sh7785lcr/sh7785lcr.c           |    5 +-
 board/sandburst/common/ppc440gx_i2c.c         |    5 +-
 board/sandburst/karef/karef.c                 |   10 ++-
 board/sandburst/metrobox/metrobox.c           |   10 ++-
 board/siemens/common/fpga.c                   |    7 +-
 board/siemens/pcu_e/pcu_e.c                   |    5 +-
 board/spear/common/spr_misc.c                 |   11 +--
 board/ssv/common/cmd_sled.c                   |    5 +-
 board/ssv/common/wd_pio.c                     |    5 +-
 board/tcm-bf537/flash.c                       |    5 +-
 board/tqc/tqm5200/cmd_stk52xx.c               |   38 +++-----
 board/tqc/tqm5200/cmd_tb5200.c                |   16 ++-
 board/tqc/tqm8272/tqm8272.c                   |    5 +-
 board/trab/cmd_trab.c                         |   48 ++++------
 board/trab/trab.c                             |    5 +-
 board/trizepsiv/eeprom.c                      |   20 ++---
 board/w7o/cmd_vpd.c                           |    8 +-
 board/zeus/update.c                           |    6 +-
 board/zeus/zeus.c                             |   13 ++-
 common/cmd_ambapp.c                           |    5 +-
 common/cmd_bdinfo.c                           |   30 ++++++-
 common/cmd_bedbug.c                           |   46 ++++++---
 common/cmd_bmp.c                              |    8 +-
 common/cmd_boot.c                             |   10 +-
 common/cmd_bootldr.c                          |    5 +-
 common/cmd_bootm.c                            |   38 ++++---
 common/cmd_cache.c                            |   18 ++--
 common/cmd_console.c                          |    5 +-
 common/cmd_cplbinfo.c                         |    5 +-
 common/cmd_cramfs.c                           |   12 ++-
 common/cmd_dataflash_mmc_mux.c                |    2 +-
 common/cmd_date.c                             |    5 +-
 common/cmd_dcr.c                              |   32 +++----
 common/cmd_df.c                               |    7 +-
 common/cmd_diag.c                             |    2 +-
 common/cmd_display.c                          |    2 +-
 common/cmd_dtt.c                              |    5 +-
 common/cmd_echo.c                             |    2 +-
 common/cmd_eeprom.c                           |    7 +-
 common/cmd_elf.c                              |   18 +++-
 common/cmd_exit.c                             |    5 +-
 common/cmd_ext2.c                             |   14 +--
 common/cmd_fat.c                              |   27 +++---
 common/cmd_fdc.c                              |    5 +-
 common/cmd_fdos.c                             |    9 +-
 common/cmd_fdt.c                              |   38 +++-----
 common/cmd_flash.c                            |   49 ++++------
 common/cmd_fpga.c                             |   13 +--
 common/cmd_help.c                             |    4 +-
 common/cmd_i2c.c                              |   76 ++++++---------
 common/cmd_ide.c                              |   25 ++---
 common/cmd_immap.c                            |   51 +++++++---
 common/cmd_irq.c                              |   10 +-
 common/cmd_itest.c                            |    8 +-
 common/cmd_jffs2.c                            |   15 +++-
 common/cmd_license.c                          |    5 +-
 common/cmd_load.c                             |   34 +++++--
 common/cmd_log.c                              |    8 +-
 common/cmd_mac.c                              |    2 +-
 common/cmd_mem.c                              |  120 +++++++++--------------
 common/cmd_mfsl.c                             |   34 +++----
 common/cmd_mgdisk.c                           |    2 +-
 common/cmd_mii.c                              |   11 +--
 common/cmd_misc.c                             |    8 +-
 common/cmd_mmc.c                              |   38 +++-----
 common/cmd_mp.c                               |   17 +--
 common/cmd_mtdparts.c                         |   13 ++-
 common/cmd_nand.c                             |   10 +-
 common/cmd_net.c                              |   45 +++++----
 common/cmd_nvedit.c                           |   37 +++----
 common/cmd_onenand.c                          |    5 +-
 common/cmd_otp.c                              |    7 +-
 common/cmd_pci.c                              |    8 +-
 common/cmd_pcmcia.c                           |    2 +-
 common/cmd_portio.c                           |   16 +--
 common/cmd_reginfo.c                          |   20 ++++-
 common/cmd_reiser.c                           |   14 +--
 common/cmd_sata.c                             |   20 ++---
 common/cmd_scsi.c                             |   21 ++---
 common/cmd_setexpr.c                          |    8 +-
 common/cmd_sf.c                               |   16 +--
 common/cmd_source.c                           |    5 +-
 common/cmd_spi.c                              |    5 +-
 common/cmd_spibootldr.c                       |    5 +-
 common/cmd_strings.c                          |    8 +-
 common/cmd_terminal.c                         |    6 +-
 common/cmd_test.c                             |    6 +-
 common/cmd_tsi148.c                           |    5 +-
 common/cmd_ubi.c                              |   23 ++---
 common/cmd_ubifs.c                            |   28 +++---
 common/cmd_universe.c                         |    5 +-
 common/cmd_usb.c                              |   12 +--
 common/cmd_version.c                          |    5 +-
 common/cmd_vfd.c                              |    8 +-
 common/cmd_ximg.c                             |    5 +-
 common/cmd_yaffs2.c                           |  130 +++++++++++++++++++------
 common/command.c                              |   37 +------
 common/hush.c                                 |    9 +-
 common/kgdb.c                                 |    2 +-
 common/lcd.c                                  |    5 +-
 common/main.c                                 |   58 +++++++----
 cpu/arm_cortexa8/mx51/clock.c                 |    2 +-
 cpu/arm_cortexa8/omap3/board.c                |    2 +-
 cpu/leon2/interrupts.c                        |    8 ++-
 cpu/leon3/interrupts.c                        |    8 ++-
 cpu/microblaze/interrupts.c                   |    3 +
 cpu/mpc512x/diu.c                             |    8 +-
 cpu/mpc512x/iim.c                             |   11 +--
 cpu/mpc512x/speed.c                           |    5 +-
 cpu/mpc5xx/interrupts.c                       |    3 +
 cpu/mpc5xxx/interrupts.c                      |    7 +-
 cpu/mpc8260/interrupts.c                      |    7 +-
 cpu/mpc83xx/ecc.c                             |   11 +--
 cpu/mpc83xx/speed.c                           |    5 +-
 cpu/nios2/epcs.c                              |    5 +-
 cpu/nios2/interrupts.c                        |    3 +
 cpu/nios2/sysid.c                             |    5 +-
 cpu/ppc4xx/cmd_chip_config.c                  |    5 +-
 cpu/ppc4xx/interrupts.c                       |    3 +
 doc/README.commands                           |    3 +-
 drivers/gpio/pca953x.c                        |   39 +++++---
 drivers/misc/ds4510.c                         |   71 +++++++++-----
 drivers/qe/qe.c                               |   11 +--
 include/command.h                             |   22 +++--
 include/common.h                              |    4 +-
 lib_blackfin/cmd_cache_dump.c                 |   10 ++-
 lib_i386/interrupts.c                         |    3 +
 202 files changed, 1604 insertions(+), 1301 deletions(-)

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [U-Boot] [RFC 1/2] cmd: Remove CONFIG_SYS_MAXARGS
  2010-03-12 15:51 [U-Boot] [RFC 0/2] Remove CONFIG_SYS_MAXARGS John Schmoller
@ 2010-03-12 15:51 ` John Schmoller
  2010-03-12 17:12 ` [U-Boot] [RFC 0/2] " Wolfgang Denk
  1 sibling, 0 replies; 3+ messages in thread
From: John Schmoller @ 2010-03-12 15:51 UTC (permalink / raw)
  To: u-boot

CONFIG_SYS_MAXARGS is an arbitrary limit on the number of arguments
which can be input at the command line. Remove that arbitrary limit.
Setting maxargs in the cmdtp to 0 will result in the ability to enter
as many arguments as you can hold in CONFIG_SYS_CBSIZE.

Signed-off-by: John Schmoller <jschmoller@xes-inc.com>
---
 board/amcc/makalu/cmd_pll.c                   |    2 +-
 board/esd/du440/du440.c                       |    4 +-
 board/fads/fads.h                             |    1 -
 board/freescale/common/pixis.c                |    4 +-
 board/freescale/mpc8610hpcd/mpc8610hpcd_diu.c |    2 +-
 board/pxa255_idp/pxa_idp.c                    |    2 +-
 common/cmd_boot.c                             |    2 +-
 common/cmd_bootm.c                            |    4 +-
 common/cmd_diag.c                             |    2 +-
 common/cmd_display.c                          |    2 +-
 common/cmd_echo.c                             |    2 +-
 common/cmd_help.c                             |    4 +-
 common/cmd_mp.c                               |    2 +-
 common/cmd_nand.c                             |    2 +-
 common/cmd_nvedit.c                           |    8 ++--
 common/cmd_onenand.c                          |    2 +-
 common/cmd_test.c                             |    6 ++--
 common/command.c                              |   36 ++-------------------
 common/hush.c                                 |    5 ++-
 common/kgdb.c                                 |    2 +-
 common/main.c                                 |   41 +++++++++++++++++++------
 cpu/arm_cortexa8/mx51/clock.c                 |    2 +-
 cpu/mpc512x/diu.c                             |    2 +-
 cpu/mpc512x/iim.c                             |    2 +-
 include/command.h                             |    2 +-
 include/common.h                              |    4 ++-
 26 files changed, 71 insertions(+), 76 deletions(-)

diff --git a/board/amcc/makalu/cmd_pll.c b/board/amcc/makalu/cmd_pll.c
index 9bae67e..5cadb4a 100644
--- a/board/amcc/makalu/cmd_pll.c
+++ b/board/amcc/makalu/cmd_pll.c
@@ -236,7 +236,7 @@ ret:
 }
 
 U_BOOT_CMD(
-	pllalter, CONFIG_SYS_MAXARGS, 1,        do_pll_alter,
+	pllalter, 0, 1,        do_pll_alter,
 	"change pll frequence",
 	"pllalter <selection>      - change pll frequence \n\n\
 	** New freq take effect after reset. ** \n\
diff --git a/board/esd/du440/du440.c b/board/esd/du440/du440.c
index 111cce5..0c3d976 100644
--- a/board/esd/du440/du440.c
+++ b/board/esd/du440/du440.c
@@ -841,7 +841,7 @@ int do_time(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	return ret;
 }
 U_BOOT_CMD(
-	time,	CONFIG_SYS_MAXARGS,	1,	do_time,
+	time,	0,	1,	do_time,
 	"run command and output execution time",
 	""
 );
@@ -891,7 +891,7 @@ int do_gfxdemo(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	return 0;
 }
 U_BOOT_CMD(
-	gfxdemo,	CONFIG_SYS_MAXARGS,	1,	do_gfxdemo,
+	gfxdemo,	0,	1,	do_gfxdemo,
 	"demo",
 	""
 );
diff --git a/board/fads/fads.h b/board/fads/fads.h
index 4ab4b26..c7d2b2d 100644
--- a/board/fads/fads.h
+++ b/board/fads/fads.h
@@ -140,7 +140,6 @@
 #define	CONFIG_SYS_CBSIZE		256		/* Console I/O Buffer Size	*/
 #endif
 #define	CONFIG_SYS_PBSIZE (CONFIG_SYS_CBSIZE + sizeof(CONFIG_SYS_PROMPT) + 16) /* Print Buffer Size	*/
-#define	CONFIG_SYS_MAXARGS		16		/* max number of command args	*/
 #define CONFIG_SYS_BARGSIZE		CONFIG_SYS_CBSIZE	/* Boot Argument Buffer Size	*/
 
 #define CONFIG_SYS_LOAD_ADDR		0x00100000
diff --git a/board/freescale/common/pixis.c b/board/freescale/common/pixis.c
index 7210512..b4b5d78 100644
--- a/board/freescale/common/pixis.c
+++ b/board/freescale/common/pixis.c
@@ -354,7 +354,7 @@ int pixis_set_sgmii(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 }
 
 U_BOOT_CMD(
-	pixis_set_sgmii, CONFIG_SYS_MAXARGS, 1, pixis_set_sgmii,
+	pixis_set_sgmii, 0, 1, pixis_set_sgmii,
 	"pixis_set_sgmii"
 	" - Enable or disable SGMII mode for a given TSEC \n",
 	"\npixis_set_sgmii [TSEC num] <on|off|switch>\n"
@@ -550,7 +550,7 @@ pixis_reset_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 
 
 U_BOOT_CMD(
-	pixis_reset, CONFIG_SYS_MAXARGS, 1, pixis_reset_cmd,
+	pixis_reset, 0, 1, pixis_reset_cmd,
 	"Reset the board using the FPGA sequencer",
 	"    pixis_reset\n"
 	"    pixis_reset [altbank]\n"
diff --git a/board/freescale/mpc8610hpcd/mpc8610hpcd_diu.c b/board/freescale/mpc8610hpcd/mpc8610hpcd_diu.c
index 4186a2e..6b8fd28 100644
--- a/board/freescale/mpc8610hpcd/mpc8610hpcd_diu.c
+++ b/board/freescale/mpc8610hpcd/mpc8610hpcd_diu.c
@@ -138,7 +138,7 @@ int mpc8610diu_init_show_bmp(cmd_tbl_t *cmdtp,
 }
 
 U_BOOT_CMD(
-	diufb, CONFIG_SYS_MAXARGS, 1, mpc8610diu_init_show_bmp,
+	diufb, 0, 1, mpc8610diu_init_show_bmp,
 	"Init or Display BMP file",
 	"init\n    - initialize DIU\n"
 	"addr\n    - display bmp at address 'addr'"
diff --git a/board/pxa255_idp/pxa_idp.c b/board/pxa255_idp/pxa_idp.c
index 05e30ec..72ef9e6 100644
--- a/board/pxa255_idp/pxa_idp.c
+++ b/board/pxa255_idp/pxa_idp.c
@@ -128,7 +128,7 @@ int do_idpcmd(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	return 0;
 }
 
-U_BOOT_CMD(idpcmd, CONFIG_SYS_MAXARGS, 0, do_idpcmd,
+U_BOOT_CMD(idpcmd, 0, 0, do_idpcmd,
 	   "custom IDP command",
 	   "no args at this time"
 );
diff --git a/common/cmd_boot.c b/common/cmd_boot.c
index bfc1db2..1aa6081 100644
--- a/common/cmd_boot.c
+++ b/common/cmd_boot.c
@@ -63,7 +63,7 @@ int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 /* -------------------------------------------------------------------- */
 
 U_BOOT_CMD(
-	go, CONFIG_SYS_MAXARGS, 1,	do_go,
+	go, 0, 1,	do_go,
 	"start application at address 'addr'",
 	"addr [arg ...]\n    - start application at address 'addr'\n"
 	"      passing 'arg' as arguments"
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index 23ab0c4..55f5a42 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -984,7 +984,7 @@ static void *boot_get_kernel (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]
 }
 
 U_BOOT_CMD(
-	bootm,	CONFIG_SYS_MAXARGS,	1,	do_bootm,
+	bootm,	0,	1,	do_bootm,
 	"boot application image from memory",
 	"[addr [arg ...]]\n    - boot application image stored in memory\n"
 	"\tpassing arguments 'arg ...'; when booting a Linux kernel,\n"
@@ -1133,7 +1133,7 @@ static int image_info (ulong addr)
 }
 
 U_BOOT_CMD(
-	iminfo,	CONFIG_SYS_MAXARGS,	1,	do_iminfo,
+	iminfo,	0,	1,	do_iminfo,
 	"print header information for application image",
 	"addr [addr ...]\n"
 	"    - print header information for application image starting at\n"
diff --git a/common/cmd_diag.c b/common/cmd_diag.c
index 0436c49..c39b16c 100644
--- a/common/cmd_diag.c
+++ b/common/cmd_diag.c
@@ -65,7 +65,7 @@ int do_diag (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 /***************************************************/
 
 U_BOOT_CMD(
-	diag,	CONFIG_SYS_MAXARGS,	0,	do_diag,
+	diag,	0,	0,	do_diag,
 	"perform board diagnostics",
 	     "    - print list of available tests\n"
 	"diag [test1 [test2]]\n"
diff --git a/common/cmd_display.c b/common/cmd_display.c
index 3422395..6851329 100644
--- a/common/cmd_display.c
+++ b/common/cmd_display.c
@@ -70,7 +70,7 @@ int do_display (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 /***************************************************/
 
 U_BOOT_CMD(
-	display,	CONFIG_SYS_MAXARGS,	1,	do_display,
+	display,	0,	1,	do_display,
 	"display string on dot matrix display",
 	"[<string>]\n"
 	"    - with <string> argument: display <string> on dot matrix display\n"
diff --git a/common/cmd_echo.c b/common/cmd_echo.c
index 3ec4d48..0f96efb 100644
--- a/common/cmd_echo.c
+++ b/common/cmd_echo.c
@@ -51,7 +51,7 @@ int do_echo(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 }
 
 U_BOOT_CMD(
-	echo,	CONFIG_SYS_MAXARGS,	1,	do_echo,
+	echo,	0,	1,	do_echo,
 	"echo args to console",
 	"[args..]\n"
 	"    - echo args to console; \\c suppresses newline"
diff --git a/common/cmd_help.c b/common/cmd_help.c
index e860dfb..c729f79 100644
--- a/common/cmd_help.c
+++ b/common/cmd_help.c
@@ -32,7 +32,7 @@ int do_help(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 }
 
 U_BOOT_CMD(
-	help,	CONFIG_SYS_MAXARGS,	1,	do_help,
+	help,	0,	1,	do_help,
 	"print command description/usage",
 	"\n"
 	"	- print brief description of all commands\n"
@@ -42,7 +42,7 @@ U_BOOT_CMD(
 
 /* This does not use the U_BOOT_CMD macro as ? can't be used in symbol names */
 cmd_tbl_t __u_boot_cmd_question_mark Struct_Section = {
-	"?",	CONFIG_SYS_MAXARGS,	1,	do_help,
+	"?",	0,	1,	do_help,
 	"alias for 'help'",
 #ifdef  CONFIG_SYS_LONGHELP
 	""
diff --git a/common/cmd_mp.c b/common/cmd_mp.c
index d78c209..b8ddbf3 100644
--- a/common/cmd_mp.c
+++ b/common/cmd_mp.c
@@ -84,7 +84,7 @@ cpu_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 #endif
 
 U_BOOT_CMD(
-	cpu, CONFIG_SYS_MAXARGS, 1, cpu_cmd,
+	cpu, 0, 1, cpu_cmd,
 	"Multiprocessor CPU boot manipulation and release",
 	    "<num> reset                 - Reset cpu <num>\n"
 	"cpu <num> status                - Status of cpu <num>\n"
diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 075a8af..430c115 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -475,7 +475,7 @@ usage:
 	return 1;
 }
 
-U_BOOT_CMD(nand, CONFIG_SYS_MAXARGS, 1, do_nand,
+U_BOOT_CMD(nand, 0, 1, do_nand,
 	"NAND sub-system",
 	"info - show available NAND devices\n"
 	"nand device [dev] - show or set current device\n"
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index eb89e9e..6020e85 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -638,7 +638,7 @@ U_BOOT_CMD(
 #endif
 
 U_BOOT_CMD(
-	printenv, CONFIG_SYS_MAXARGS, 1,	do_printenv,
+	printenv, 0, 1,	do_printenv,
 	"print environment variables",
 	"\n    - print values of all environment variables\n"
 	"printenv name ...\n"
@@ -646,7 +646,7 @@ U_BOOT_CMD(
 );
 
 U_BOOT_CMD(
-	setenv, CONFIG_SYS_MAXARGS, 0,	do_setenv,
+	setenv, 0, 0,	do_setenv,
 	"set environment variables",
 	"name value ...\n"
 	"    - set environment variable 'name' to 'value ...'\n"
@@ -657,7 +657,7 @@ U_BOOT_CMD(
 #if defined(CONFIG_CMD_ASKENV)
 
 U_BOOT_CMD(
-	askenv,	CONFIG_SYS_MAXARGS,	1,	do_askenv,
+	askenv,	0,	1,	do_askenv,
 	"get environment variables from stdin",
 	"name [message] [size]\n"
 	"    - get environment variable 'name' from stdin (max 'size' chars)\n"
@@ -674,7 +674,7 @@ U_BOOT_CMD(
 #if defined(CONFIG_CMD_RUN)
 int do_run (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
 U_BOOT_CMD(
-	run,	CONFIG_SYS_MAXARGS,	1,	do_run,
+	run,	0,	1,	do_run,
 	"run commands in an environment variable",
 	"var [...]\n"
 	"    - run the commands in the environment variable(s) 'var'"
diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c
index 565257c..7701e21 100644
--- a/common/cmd_onenand.c
+++ b/common/cmd_onenand.c
@@ -481,7 +481,7 @@ usage:
 }
 
 U_BOOT_CMD(
-	onenand,	CONFIG_SYS_MAXARGS,	1,	do_onenand,
+	onenand,	0,	1,	do_onenand,
 	"OneNAND sub-system",
 	"info - show available OneNAND devices\n"
 	"onenand bad - show bad blocks\n"
diff --git a/common/cmd_test.c b/common/cmd_test.c
index d886f89..88ed2ba 100644
--- a/common/cmd_test.c
+++ b/common/cmd_test.c
@@ -145,7 +145,7 @@ int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 }
 
 U_BOOT_CMD(
-	test,	CONFIG_SYS_MAXARGS,	1,	do_test,
+	test,	0,	1,	do_test,
 	"minimal test like /bin/sh",
 	"[args..]"
 );
@@ -156,7 +156,7 @@ int do_false(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 }
 
 U_BOOT_CMD(
-	false,	CONFIG_SYS_MAXARGS,	1,	do_false,
+	false,	0,	1,	do_false,
 	"do nothing, unsuccessfully",
 	NULL
 );
@@ -167,7 +167,7 @@ int do_true(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 }
 
 U_BOOT_CMD(
-	true,	CONFIG_SYS_MAXARGS,	1,	do_true,
+	true,	0,	1,	do_true,
 	"do nothing, successfully",
 	NULL
 );
diff --git a/common/command.c b/common/command.c
index 0c66b7a..f0db2ed 100644
--- a/common/command.c
+++ b/common/command.c
@@ -268,36 +268,6 @@ static int complete_cmdv(int argc, char *argv[], char last_char, int maxv, char
 	return n_found;
 }
 
-static int make_argv(char *s, int argvsz, char *argv[])
-{
-	int argc = 0;
-
-	/* split into argv */
-	while (argc < argvsz - 1) {
-
-		/* skip any white space */
-		while ((*s == ' ') || (*s == '\t'))
-			++s;
-
-		if (*s == '\0')	/* end of s, no more args	*/
-			break;
-
-		argv[argc++] = s;	/* begin of argument string	*/
-
-		/* find end of string */
-		while (*s && (*s != ' ') && (*s != '\t'))
-			++s;
-
-		if (*s == '\0')		/* end of s, no more args	*/
-			break;
-
-		*s++ = '\0';		/* terminate current arg	 */
-	}
-	argv[argc] = NULL;
-
-	return argc;
-}
-
 static void print_argv(const char *banner, const char *leader, const char *sep, int linemax, char *argv[])
 {
 	int ll = leader != NULL ? strlen(leader) : 0;
@@ -352,7 +322,7 @@ static char tmp_buf[CONFIG_SYS_CBSIZE];	/* copy of console I/O buffer	*/
 int cmd_auto_complete(const char *const prompt, char *buf, int *np, int *colp)
 {
 	int n = *np, col = *colp;
-	char *argv[CONFIG_SYS_MAXARGS + 1];		/* NULL terminated	*/
+	char **argv;		/* NULL terminated	*/
 	char *cmdv[20];
 	char *s, *t;
 	const char *sep;
@@ -373,7 +343,9 @@ int cmd_auto_complete(const char *const prompt, char *buf, int *np, int *colp)
 	strcpy(tmp_buf, buf);
 
 	/* separate into argv */
-	argc = make_argv(tmp_buf, sizeof(argv)/sizeof(argv[0]), argv);
+	argc = arg_count(tmp_buf);
+	*argv = (char *)malloc(sizeof(char *) * argc);
+	parse_line(tmp_buf, argv, argc);
 
 	/* do the completion and return the possible completions */
 	i = complete_cmdv(argc, argv, last_char, sizeof(cmdv)/sizeof(cmdv[0]), cmdv);
diff --git a/common/hush.c b/common/hush.c
index 06c5ff8..1837a7f 100644
--- a/common/hush.c
+++ b/common/hush.c
@@ -1694,7 +1694,8 @@ static int run_pipe_real(struct pipe *pi)
 				}
 #endif
 				/* found - check max args */
-				if ((child->argc - i) > cmdtp->maxargs) {
+				if (cmdtp->maxargs &&
+				    (child->argc - i) > cmdtp->maxargs) {
 					cmd_usage(cmdtp);
 					return -1;
 				}
@@ -3627,7 +3628,7 @@ int do_showvar (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 }
 
 U_BOOT_CMD(
-	showvar, CONFIG_SYS_MAXARGS, 1,	do_showvar,
+	showvar, 0, 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 0531452..ac47c59 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, CONFIG_SYS_MAXARGS, 1,	do_kgdb,
+	kgdb, 0, 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/main.c b/common/main.c
index 7ac657f..e767694 100644
--- a/common/main.c
+++ b/common/main.c
@@ -30,9 +30,7 @@
 #include <common.h>
 #include <watchdog.h>
 #include <command.h>
-#ifdef CONFIG_MODEM_SUPPORT
 #include <malloc.h>		/* for free() prototype */
-#endif
 
 #ifdef CONFIG_SYS_HUSH_PARSER
 #include <hush.h>
@@ -1112,15 +1110,35 @@ static char * delete_char (char *buffer, char *p, int *colp, int *np, int plen)
 }
 
 /****************************************************************************/
+int arg_count(char *line)
+{
+	int argc = 0;
+
+	while (*line != '\0') {
+
+		while ((*line == ' ') || (*line == '\t')) {
+			++line;
+		}
 
-int parse_line (char *line, char *argv[])
+		argc++;
+
+		/* find end of string */
+		while (*line && (*line != ' ') && (*line != '\t')) {
+			++line;
+		}
+	}
+
+	return argc;
+}
+
+int parse_line (char *line, char *argv[], uint argc)
 {
 	int nargs = 0;
 
 #ifdef DEBUG_PARSER
 	printf ("parse_line: \"%s\"\n", line);
 #endif
-	while (nargs < CONFIG_SYS_MAXARGS) {
+	while (nargs < argc) {
 
 		/* skip any white space */
 		while ((*line == ' ') || (*line == '\t')) {
@@ -1153,8 +1171,6 @@ int parse_line (char *line, char *argv[])
 		*line++ = '\0';		/* terminate current arg	 */
 	}
 
-	printf ("** Too many args (max. %d) **\n", CONFIG_SYS_MAXARGS);
-
 #ifdef DEBUG_PARSER
 	printf ("parse_line: nargs=%d\n", nargs);
 #endif
@@ -1298,8 +1314,9 @@ int run_command (const char *cmd, int flag)
 	char *sep;			/* end of token (separator) in cmdbuf */
 	char finaltoken[CONFIG_SYS_CBSIZE];
 	char *str = cmdbuf;
-	char *argv[CONFIG_SYS_MAXARGS + 1];	/* NULL terminated	*/
-	int argc, inquotes;
+	char **argv;
+	uint argc;
+	int inquotes;
 	int repeatable = 1;
 	int rc = 0;
 
@@ -1365,7 +1382,9 @@ int run_command (const char *cmd, int flag)
 		process_macros (token, finaltoken);
 
 		/* Extract arguments */
-		if ((argc = parse_line (finaltoken, argv)) == 0) {
+		argc = arg_count(finaltoken);
+		argv = (char **)malloc(sizeof(char *) * argc);
+		if (parse_line (finaltoken, argv, argc) == 0) {
 			rc = -1;	/* no command@all */
 			continue;
 		}
@@ -1378,7 +1397,7 @@ int run_command (const char *cmd, int flag)
 		}
 
 		/* found - check max args */
-		if (argc > cmdtp->maxargs) {
+		if (cmdtp->maxargs && argc > cmdtp->maxargs) {
 			cmd_usage(cmdtp);
 			rc = -1;
 			continue;
@@ -1407,6 +1426,8 @@ int run_command (const char *cmd, int flag)
 
 		repeatable &= cmdtp->repeatable;
 
+		free(argv);
+
 		/* Did the user stop this? */
 		if (had_ctrlc ())
 			return -1;	/* if stopped then not repeatable */
diff --git a/cpu/arm_cortexa8/mx51/clock.c b/cpu/arm_cortexa8/mx51/clock.c
index 38480ac..b594caf 100644
--- a/cpu/arm_cortexa8/mx51/clock.c
+++ b/cpu/arm_cortexa8/mx51/clock.c
@@ -288,7 +288,7 @@ int do_mx51_showclocks(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 /***************************************************/
 
 U_BOOT_CMD(
-	clockinfo,	CONFIG_SYS_MAXARGS,	1,	do_mx51_showclocks,
+	clockinfo,	0,	1,	do_mx51_showclocks,
 	"display mx51 clocks\n",
 	""
 );
diff --git a/cpu/mpc512x/diu.c b/cpu/mpc512x/diu.c
index a24f395..bbb8421 100644
--- a/cpu/mpc512x/diu.c
+++ b/cpu/mpc512x/diu.c
@@ -126,7 +126,7 @@ int mpc5121diu_init_show_bmp(cmd_tbl_t *cmdtp,
 }
 
 U_BOOT_CMD(
-	diufb, CONFIG_SYS_MAXARGS, 1, mpc5121diu_init_show_bmp,
+	diufb, 0, 1, mpc5121diu_init_show_bmp,
 	"Init or Display BMP file",
 	"init\n    - initialize DIU\n"
 	"addr\n    - display bmp at address 'addr'"
diff --git a/cpu/mpc512x/iim.c b/cpu/mpc512x/iim.c
index 8f2eb37..ae3de0b 100644
--- a/cpu/mpc512x/iim.c
+++ b/cpu/mpc512x/iim.c
@@ -374,7 +374,7 @@ int do_ads5121_fuse(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 }
 
 U_BOOT_CMD(
-	fuse, CONFIG_SYS_MAXARGS, 0, do_ads5121_fuse,
+	fuse, 0, 0, do_ads5121_fuse,
 	"   - Read, Sense, Override or Program Fuses\n",
 	"bank <n>		- sets active Fuse Bank to 0 or 1\n"
 	"			    no args shows current active bank\n"
diff --git a/include/command.h b/include/command.h
index 55caa6e..a40586d 100644
--- a/include/command.h
+++ b/include/command.h
@@ -45,7 +45,7 @@
 
 struct cmd_tbl_s {
 	char		*name;		/* Command Name			*/
-	int		maxargs;	/* maximum number of arguments	*/
+	uint		maxargs;	/* maximum number of arguments	*/
 	int		repeatable;	/* autorepeat allowed?		*/
 					/* Implementation function	*/
 	int		(*cmd)(struct cmd_tbl_s *, int, int, char *[]);
diff --git a/include/common.h b/include/common.h
index a133e34..6c15c02 100644
--- a/include/common.h
+++ b/include/common.h
@@ -1,3 +1,4 @@
+
 /*
  * (C) Copyright 2000-2009
  * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
@@ -226,7 +227,8 @@ void	main_loop	(void);
 int	run_command	(const char *cmd, int flag);
 int	readline	(const char *const prompt);
 int	readline_into_buffer	(const char *const prompt, char * buffer);
-int	parse_line (char *, char *[]);
+int	arg_count(char *);
+int	parse_line (char *, char *[], uint);
 void	init_cmd_timeout(void);
 void	reset_cmd_timeout(void);
 
-- 
1.6.0.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [U-Boot] [RFC 0/2] Remove CONFIG_SYS_MAXARGS
  2010-03-12 15:51 [U-Boot] [RFC 0/2] Remove CONFIG_SYS_MAXARGS John Schmoller
  2010-03-12 15:51 ` [U-Boot] [RFC 1/2] cmd: " John Schmoller
@ 2010-03-12 17:12 ` Wolfgang Denk
  1 sibling, 0 replies; 3+ messages in thread
From: Wolfgang Denk @ 2010-03-12 17:12 UTC (permalink / raw)
  To: u-boot

Dear John Schmoller,

In message <cover.1268408350.git.jschmoller@xes-inc.com> you wrote:
> The first patch removes CONFIG_SYS_MAXARGS, replacing the staticly defined
> array with a malloc'd array of the appropriate size. When a function has no
> upper argument limit (ie, was set to CONFIG_SYS_MAXARGS), it is now set to 0
> to indicate this fact.  Argument count is now unlimited, within reason and
> malloc buffer size.
> 
> The second patch removes cmdtp->maxargs and moves the checks to the individual
> command functions.  Since most functions do bounds checking anyway, it's a
> a fairly cheap task (sometimes free) to remove this bounds check in
> common/main.c.  In addition, it's more intuitive (in my opinion) if all bounds
> checking is done in only one place for each function.  The second patch also
> creates a CMD_ERR_USAGE return value, which prints usage when returned.  The
> overall effect of this patch is to reduce code size by an average of 200-250
> bytes and, I feel, make things a bit cleaner.

Hm... I would have expected that the code grows... Interesting...

> I'm looking for comments on these two patches as they are quite invasive, and
> will definitly cause problems for those people who maintain their own code
> out-of-tree.  They may also require an additional amount of testing.
> 
> John Schmoller (2):
>   cmd: Remove CONFIG_SYS_MAXARGS
>   command: Remove maxargs from command structure

This patch is way too big for the maximim list size. For the
purpose of discussing the implementation we probably don;t need to see
all changes to all affected iles - the interesting parts are the
header files, the common files like  common/command.c,  common/hush.c,
common/kgdb.c,  common/lcd.c, common/main.c,  and say a few (3...5) co
the commands (common/cmd_*.c).  That should give us enough context to
review the code.

Of course you can (additionally) put the full patch on some server so
we can actually test it...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It all seemed, he thought, to be rather a lot of  trouble  to  go  to
just sharpen a razor blade.  - Terry Pratchett, _The Light Fantastic_

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-03-12 17:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-12 15:51 [U-Boot] [RFC 0/2] Remove CONFIG_SYS_MAXARGS John Schmoller
2010-03-12 15:51 ` [U-Boot] [RFC 1/2] cmd: " John Schmoller
2010-03-12 17:12 ` [U-Boot] [RFC 0/2] " Wolfgang Denk

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.