[U-Boot] [PATCH v5] [RESEND] Add imls utility command

[Wolfgang Denk]

Dear Marco,

In message <49E61F99.50001 at gmail.com> you wrote:
> From: Marco Stornelli <marco.stornelli at gmail.com>
> 
> This patch adds, under tools folder, a new command called imls. Its
> goal is the same of UBoot's imls but it can be used as Linux shell
> command. It reads from raw mtd partition and prints the list of the
> stored images.

Hm...

...
> +	while (--argc > 0 && **++argv == '-') {
> +		while (*++*argv) {
> +			switch (**argv) {
> +			case 's':
> +				if (--argc <= 0) 
> +					usage ();
> +				sectorcount = (unsigned int)atoi(*++argv);
> +				sflag = 1;
> +				goto NXTARG;
> +			case 'o':
> +				if (--argc <= 0)
> +					usage ();
> +				sectoroffset = (unsigned int)atoi(*++argv);
> +				goto NXTARG;
> +
> +			case 'c':
> +				if (--argc <= 0)
> +					usage ();
> +				sectorsize = (unsigned int)atoi(*++argv);
> +				cflag = 1;
> +				goto NXTARG;
> +			default:
> +				usage ();
...
> +	fprintf (stderr, "Usage:\n"
> +			 "       %s [-o offset] -c size -s count device\n"
> +			 "          -o ==> number of sectors to use as offset\n"
> +			 "          -s ==> number of sectors\n"
> +			 "          -c ==> size of sectors (byte)\n",
> +		cmdname);

I think this is *extremely" misleading.

If I have options "-c" and "-s" and arguments "size" and "count" I
will *always" assume that "-s" is for "size" and "-c" is for "count".

Please consider the principle of least surprise
(http://en.wikipedia.org/wiki/Least_surprise) and swap variable resp.
option names.

Thanks.

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
What is research but a blind date with knowledge?      -- Will Harvey