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

Jeroen Hofstee
2014-11-13T21:42:31Z

Hello Andreas,

On 27-10-14 13:06, Jeroen Hofstee wrote:
> Hello Andreas,
>
> On 24-10-14 23:25, andreas.devel at 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