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

[Marco Stornelli]

2009/4/28 Wolfgang Denk <wd at denx.de>:
> 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".

-s was for "Sector count" but it can be used even as "size", it's a
choice. However it's a very simple modification, I'll resend a new
version as soon as possible.

Marco

>
> 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
>