[U-Boot] [PATCH] tools/kwbimage.c: fix parser error handling

From: Andreas Bießmann andreas.devel@googlemail.com
The two error checks for image_boot_mode_id and image_nand_ecc_mode_id where wrong and would never fail, fix that!
This was detected by Apple's clang compiler: ---8<--- HOSTCC tools/kwbimage.o tools/kwbimage.c:553:20: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare] if (el->bootfrom < 0) { ~~~~~~~~~~~~ ^ ~ tools/kwbimage.c:571:23: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare] if (el->nandeccmode < 0) { ~~~~~~~~~~~~~~~ ^ ~ 2 warnings generated. --->8---
Signed-off-by: Andreas Bießmann andreas.devel@googlemail.com ---
tools/kwbimage.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 42870ed..8fd70ef 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -548,13 +548,14 @@ static int image_create_config_parse_oneline(char *line, el->version = atoi(value); } else if (!strcmp(keyword, "BOOT_FROM")) { char *value = strtok_r(NULL, deliminiters, &saveptr); - el->type = IMAGE_CFG_BOOT_FROM; - el->bootfrom = image_boot_mode_id(value); - if (el->bootfrom < 0) { + int ret = image_boot_mode_id(value); + if (ret < 0) { fprintf(stderr, "Invalid boot media '%s'\n", value); return -1; } + el->type = IMAGE_CFG_BOOT_FROM; + el->bootfrom = ret; } else if (!strcmp(keyword, "NAND_BLKSZ")) { char *value = strtok_r(NULL, deliminiters, &saveptr); el->type = IMAGE_CFG_NAND_BLKSZ; @@ -566,13 +567,14 @@ static int image_create_config_parse_oneline(char *line, strtoul(value, NULL, 16); } else if (!strcmp(keyword, "NAND_ECC_MODE")) { char *value = strtok_r(NULL, deliminiters, &saveptr); - el->type = IMAGE_CFG_NAND_ECC_MODE; - el->nandeccmode = image_nand_ecc_mode_id(value); - if (el->nandeccmode < 0) { + int ret = image_nand_ecc_mode_id(value); + if (ret < 0) { fprintf(stderr, "Invalid NAND ECC mode '%s'\n", value); return -1; } + el->type = IMAGE_CFG_NAND_ECC_MODE; + el->nandeccmode = ret; } else if (!strcmp(keyword, "NAND_PAGE_SIZE")) { char *value = strtok_r(NULL, deliminiters, &saveptr); el->type = IMAGE_CFG_NAND_PAGESZ;

Hello Andreas,
On 24-10-14 23:25, andreas.devel@googlemail.com wrote:
From: Andreas Bießmann andreas.devel@googlemail.com
The two error checks for image_boot_mode_id and image_nand_ecc_mode_id where wrong and would never fail, fix that!
This was detected by Apple's clang compiler: ---8<--- HOSTCC tools/kwbimage.o tools/kwbimage.c:553:20: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare] if (el->bootfrom < 0) { ~~~~~~~~~~~~ ^ ~ tools/kwbimage.c:571:23: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare] if (el->nandeccmode < 0) { ~~~~~~~~~~~~~~~ ^ ~ 2 warnings generated. --->8---
Signed-off-by: Andreas Bießmann andreas.devel@googlemail.com
tools/kwbimage.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 42870ed..8fd70ef 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -548,13 +548,14 @@ static int image_create_config_parse_oneline(char *line, el->version = atoi(value); } else if (!strcmp(keyword, "BOOT_FROM")) { char *value = strtok_r(NULL, deliminiters, &saveptr);
el->type = IMAGE_CFG_BOOT_FROM;
el->bootfrom = image_boot_mode_id(value);
if (el->bootfrom < 0) {
int ret = image_boot_mode_id(value);
}if (ret < 0) { fprintf(stderr, "Invalid boot media '%s'\n", value); return -1;
el->type = IMAGE_CFG_BOOT_FROM;
} else if (!strcmp(keyword, "NAND_BLKSZ")) { char *value = strtok_r(NULL, deliminiters, &saveptr); el->type = IMAGE_CFG_NAND_BLKSZ;el->bootfrom = ret;
@@ -566,13 +567,14 @@ static int image_create_config_parse_oneline(char *line, strtoul(value, NULL, 16); } else if (!strcmp(keyword, "NAND_ECC_MODE")) { char *value = strtok_r(NULL, deliminiters, &saveptr);
el->type = IMAGE_CFG_NAND_ECC_MODE;
el->nandeccmode = image_nand_ecc_mode_id(value);
if (el->nandeccmode < 0) {
int ret = image_nand_ecc_mode_id(value);
Thanks for fixing this. Could you move the int ret declaration before actual code though?
Regards, Jeroen

Hello Andreas,
On 27-10-14 13:06, Jeroen Hofstee wrote:
Hello Andreas,
On 24-10-14 23:25, andreas.devel@googlemail.com wrote:
From: Andreas Bießmann andreas.devel@googlemail.com
The two error checks for image_boot_mode_id and image_nand_ecc_mode_id where wrong and would never fail, fix that!
This was detected by Apple's clang compiler: ---8<--- HOSTCC tools/kwbimage.o tools/kwbimage.c:553:20: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare] if (el->bootfrom < 0) { ~~~~~~~~~~~~ ^ ~ tools/kwbimage.c:571:23: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare] if (el->nandeccmode < 0) { ~~~~~~~~~~~~~~~ ^ ~ 2 warnings generated. --->8---
Signed-off-by: Andreas Bießmann andreas.devel@googlemail.com
tools/kwbimage.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 42870ed..8fd70ef 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -548,13 +548,14 @@ static int image_create_config_parse_oneline(char *line, el->version = atoi(value); } else if (!strcmp(keyword, "BOOT_FROM")) { char *value = strtok_r(NULL, deliminiters, &saveptr);
el->type = IMAGE_CFG_BOOT_FROM;
el->bootfrom = image_boot_mode_id(value);
if (el->bootfrom < 0) {
int ret = image_boot_mode_id(value);
if (ret < 0) { fprintf(stderr, "Invalid boot media '%s'\n", value); return -1; }
el->type = IMAGE_CFG_BOOT_FROM;
el->bootfrom = ret; } else if (!strcmp(keyword, "NAND_BLKSZ")) { char *value = strtok_r(NULL, deliminiters, &saveptr); el->type = IMAGE_CFG_NAND_BLKSZ;
@@ -566,13 +567,14 @@ static int image_create_config_parse_oneline(char *line, strtoul(value, NULL, 16); } else if (!strcmp(keyword, "NAND_ECC_MODE")) { char *value = strtok_r(NULL, deliminiters, &saveptr);
el->type = IMAGE_CFG_NAND_ECC_MODE;
el->nandeccmode = image_nand_ecc_mode_id(value);
if (el->nandeccmode < 0) {
int ret = image_nand_ecc_mode_id(value);
Thanks for fixing this. Could you move the int ret declaration before actual code though?
Ah, I see now, you were not adding the declaration in the middle of the code. There is just a newline missing after the declaration, which make the patch look like it does. This was already the case in the original code, so
Acked-By: Jeroen Hofstee jeroen@myspectrum.nl
Regards, Jeroen

ping?
It is http://patchwork.ozlabs.org/patch/403237/
On 13.11.14 22:42, Jeroen Hofstee wrote:
Hello Andreas,
On 27-10-14 13:06, Jeroen Hofstee wrote:
Hello Andreas,
On 24-10-14 23:25, andreas.devel@googlemail.com wrote:
From: Andreas Bießmann andreas.devel@googlemail.com
The two error checks for image_boot_mode_id and image_nand_ecc_mode_id where wrong and would never fail, fix that!
This was detected by Apple's clang compiler: ---8<--- HOSTCC tools/kwbimage.o tools/kwbimage.c:553:20: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare] if (el->bootfrom < 0) { ~~~~~~~~~~~~ ^ ~ tools/kwbimage.c:571:23: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare] if (el->nandeccmode < 0) { ~~~~~~~~~~~~~~~ ^ ~ 2 warnings generated. --->8---
Signed-off-by: Andreas Bießmann andreas.devel@googlemail.com
tools/kwbimage.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 42870ed..8fd70ef 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -548,13 +548,14 @@ static int image_create_config_parse_oneline(char *line, el->version = atoi(value); } else if (!strcmp(keyword, "BOOT_FROM")) { char *value = strtok_r(NULL, deliminiters, &saveptr);
el->type = IMAGE_CFG_BOOT_FROM;
el->bootfrom = image_boot_mode_id(value);
if (el->bootfrom < 0) {
int ret = image_boot_mode_id(value);
if (ret < 0) { fprintf(stderr, "Invalid boot media '%s'\n", value); return -1; }
el->type = IMAGE_CFG_BOOT_FROM;
el->bootfrom = ret; } else if (!strcmp(keyword, "NAND_BLKSZ")) { char *value = strtok_r(NULL, deliminiters, &saveptr); el->type = IMAGE_CFG_NAND_BLKSZ;
@@ -566,13 +567,14 @@ static int image_create_config_parse_oneline(char *line, strtoul(value, NULL, 16); } else if (!strcmp(keyword, "NAND_ECC_MODE")) { char *value = strtok_r(NULL, deliminiters, &saveptr);
el->type = IMAGE_CFG_NAND_ECC_MODE;
el->nandeccmode = image_nand_ecc_mode_id(value);
if (el->nandeccmode < 0) {
int ret = image_nand_ecc_mode_id(value);
Thanks for fixing this. Could you move the int ret declaration before actual code though?
Ah, I see now, you were not adding the declaration in the middle of the code. There is just a newline missing after the declaration, which make the patch look like it does. This was already the case in the original code, so
Acked-By: Jeroen Hofstee jeroen@myspectrum.nl
Regards, Jeroen

On Fri, Oct 24, 2014 at 11:25:52PM +0200, Andreas Bießmann wrote:
From: Andreas Bießmann andreas.devel@googlemail.com
The two error checks for image_boot_mode_id and image_nand_ecc_mode_id where wrong and would never fail, fix that!
This was detected by Apple's clang compiler:
Applied to u-boot/master, thanks!
participants (4)
-
Andreas Bießmann
-
andreas.devel@googlemail.com
-
Jeroen Hofstee
-
Tom Rini