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

Dear Marco,

In message 49E61F99.50001@gmail.com you wrote:

From: Marco Stornelli marco.stornelli@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