| Message ID | 1455502619-16093-17-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Simon, On Mon, Feb 15, 2016 at 10:16 AM, Simon Glass <sjg@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@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: > type_name = "null"; > break; > } > -- Regards, Bin
Hi Bin, On 16 February 2016 at 07:51, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Mon, Feb 15, 2016 at 10:16 AM, Simon Glass <sjg@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@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
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); 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: type_name = "null"; break; }
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@chromium.org> --- cmd/cbfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)