Hi Wolfgang,

On 27.10.2014 13:57, Wolfgang Denk wrote:
tools/kwbimage.c: In function ‘kwbimage_set_header’:
tools/kwbimage.c:784:8: warning: ‘headersz’ may be used uninitialized in this 
function [-Wmaybe-uninitialized]
   memcpy(ptr, image, headersz);
         ^
Instead of using multiple if statements, use a switch statement with
a default entry. And return with error if the VERSION field is
missing in the cfg file.

Thanks, but...

+       switch (version) {
+               /*
+                * Fallback to version 0 is no version is provided in the
+                * cfg file
+                */
+       case -1:
+       case 0:
                image = image_create_v0(&headersz, params, sbuf->st_size);
+               break;
+
+       case 1:
                image = image_create_v1(&headersz, params, sbuf->st_size);
+               break;
+
+       default:
+               fprintf(stderr, "File %s does not have the VERSION field\n",
+                       params->imagename);
+               free(image_cfg);
+               exit(EXIT_FAILURE);
+       }

What exactly is the difference between return code -1 (no version is
provided and you fall back to using version 0), and the default case?

To me these look the same?

Perhaps the error message is a bit misleading. The "default:" case is for unsupported versions. E.g. if "VERSION 3" would have been provided in the cfg file.

Should I rephrase the error message in the next patch version? To something like this:

        fprintf(stderr, "Unsupported VERSION %d\n", version);

Thanks,
Stefan

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to