From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?utf-8?Q?Beno=C3=AEt_Th=C3=A9baudeau?= <benoit.thebaudeau@advansee.com>
Date: Tue, 30 Oct 2012 22:59:47 +0100 (CET)
Subject: [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary,
 implement ls and fsload commands
In-Reply-To: <5090440B.3070707@wwwdotorg.org>
Message-ID: <761065953.288586.1351634387744.JavaMail.root@advansee.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Tuesday, October 30, 2012 10:18:03 PM, Stephen Warren wrote:
> On 10/30/2012 02:23 PM, Beno?t Th?baudeau wrote:
> >> +int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> >> argv[],
> >> +	int fstype)
> >> +{
> >> +	if (argc < 2)
> >> +		return CMD_RET_USAGE;
> >> +
> >> +	if (fs_set_blk_dev(argv[1], (argc >= 3) ? argv[2] : NULL,
> >> fstype))
> >> +		return 1;
> >> +
> >> +	if (fs_ls(argc == 4 ? argv[3] : "/"))
> > 
> > IMHO, it would be better to just ignore the possible extra
> > arguments, like in:
> > +	if (fs_ls(argc >= 4 ? argv[3] : "/"))
> 
> Here I don't agree. If the command expects a certain set of
> arguments,
> we should validate that the user provided exactly that set, and no
> more.
> If we allow arbitrary cruft, then if we need to add new arguments
> later,
> we won't be able to guarantee that handling those extra arguments
> won't
> break some existing broken usage of the command.

My comment was misleading. Actually, with the current code, do_ls() can not be
called (except directly) if there are more than 4 arguments, because of the way
the ls command is declared through U_BOOT_CMD(). Hence, if ">=" is used,
arguments can be added later without changing existing lines.

And if we consider a direct call to do_ls() skipping the command system, then
this function should return CMD_RET_USAGE if argc > 4.

Best regards,
Beno?t