[U-Boot] [PATCH 16/30] dm: cbfs: Fix handling of invalid type

[Simon Glass]

Hi Bin,

On 16 February 2016 at 07:51, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Mon, Feb 15, 2016 at 10:16 AM, Simon Glass <sjg at chromium.org> wrote:
>> The comment for file_cbfs_type() says that it returns 0 for an invalid type.
>> The code appears to check for -1, except that it uses an unsigned variable
>> to store the type. This results in a warning on 64-bit machines.
>>
>> Adjust it to make the meaning clearer. Continue to handle the -1 case since
>> it may be needed.
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>>  cmd/cbfs.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/cmd/cbfs.c b/cmd/cbfs.c
>> index 35d8a7a..cdfc9b6 100644
>> --- a/cmd/cbfs.c
>> +++ b/cmd/cbfs.c
>> @@ -103,7 +103,7 @@ int do_cbfs_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
>>         printf("     size              type  name\n");
>>         printf("------------------------------------------\n");
>>         while (file) {
>> -               u32 type = file_cbfs_type(file);
>> +               int type = file_cbfs_type(file);
>
> but file_cbfs_type() returns u32 as its type..
>
>>                 char *type_name = NULL;
>>                 const char *filename = file_cbfs_name(file);
>>
>> @@ -140,7 +140,7 @@ int do_cbfs_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
>>                 case CBFS_COMPONENT_CMOS_LAYOUT:
>>                         type_name = "cmos layout";
>>                         break;
>> -               case -1UL:
>> +               case -1:
>
> What about: case (u32)-1UL:

Actually they are the same thing. This code is pretty horrible - I
suspect the return value of -1 is intended, but since it is u32 they
use -1UL. But there is no need for that really.

>
>>                         type_name = "null";
>>                         break;
>>                 }
>> --

Regards,
Simon