tools/kwbimage: Fix compilation warning
PATCH 1/1 tools/kwbimage: Fix compilation warning
This patch fixes a compilation warning of kwbimage.c:
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.
--- tools/kwbimage.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
tools/kwbimage.c +18 -6
@@ -752,14 +752,26 @@ static void kwbimage_set_header(void *ptr, struct stat *sbuf, int ifd, } version =image_get_version (); -/* Fallback to version 0 is no version is provided in the cfg file */ -if (version ==-1 ) - version =0 ; - -if (version ==0 ) +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); -else if (version ==1 ) +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 ); + }if (!image) {fprintf (stderr,"Could not create image\n" );
Signature · 1 line
2.1.2
PATCH 1/1 tools/kwbimage: Fix compilation warning
Dear Stefan,
Quoted 9 lines · In message <[email protected]> you wrote:
This patch fixes a compilation warning of
kwbimage.c:
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...
Quoted 10 lines
tools/kwbimage.c +9
-0+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 ; +
tools/kwbimage.c static void kwbimage_set_header(void *ptr, struct stat *sbuf, int ifd, · line 774
Quoted 10 lines
tools/kwbimage.c +9
-0+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?
Best regards,
Wolfgang Denk
Signature · 5 lines
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
Wenn Du ein' weise Antwort verlangst, Mu?t Du vern?nftig fragen.
-- Goethe, InvektivenPATCH 1/1 tools/kwbimage: Fix compilation warning
Hi Wolfgang,
Quoted 34 lines · Wolfgang Denk
Quoted 7 lines
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...
Quoted 10 lines
tools/kwbimage.c +9
-0+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 ; +tools/kwbimage.c static void kwbimage_set_header(void *ptr, struct stat *sbuf, int ifd, · line 774
Quoted 10 lines
tools/kwbimage.c +9
-0+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?
Quoted 1 line
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
PATCH 1/1 tools/kwbimage: Fix compilation warning
Dear Stefan,
Quoted 7 lines · In message <[email protected]> you wrote:
Quoted 7 lines
Quoted 7 lines
tools/kwbimage.c +6
-0+switch (version) { +/* +* Fallback to version 0 is no version is provided in the +* cfg file +*/ +case -1 :
...
Quoted 11 lines
Quoted 4 lines
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:
In this case not the error message is incorrect, but the comment above is misleading. Instead of "no version is provided" it should probably read "if no supported version is provided" (note: please also fix the typo, i.e. "if" instead of "is").
Best regards,
Wolfgang Denk
Signature · 4 lines
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 Anything free is worth what you pay for it.
PATCH 1/1 tools/kwbimage: Fix compilation warning
Hi Wolfgang,
Quoted 17 lines · Wolfgang Denk
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
tools/kwbimage.c +6
-0+switch (version) { +/* +* Fallback to version 0 is no version is provided in the +* cfg file +*/ +case -1 :...
Quoted 9 lines
Quoted 4 lines
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.
Quoted 7 lines
Quoted 2 lines
Should I rephrase the error message in the next patch version? To something like this:
In this case not the error message is incorrect, but the comment above is misleading. Instead of "no version is provided" it should probably read "if no supported version is provided" (note: please also fix the typo, i.e. "if" instead of "is").
The fallback in the comment above refers to "version == -1". As the function image_get_version() returns -1 if no VERSION string is found in the config file. So I think the comment is quite accurate.
Thanks,
Stefan