[U-Boot] [RFC PATCH v4 1/3] sandbox: Fix compiling warning on 32-bit host

york sun york.sun at nxp.com
Tue Feb 23 17:48:25 CET 2016


On 02/16/2016 08:01 AM, Simon Glass wrote:
> +Masahiro
> 
> Hi York,
> 
> On 12 February 2016 at 13:59, York Sun <york.sun at nxp.com> wrote:
>> Fix the following compiling warning on 32-bit host
>>
>> disk/part_efi.c: In function 'alloc_read_gpt_entries':
>> disk/part_efi.c:894:2: warning: format '%zu' expects argument of
>>  type 'size_t', but argument 5 has type 'long unsigned int'
>>  [-Wformat]
>> disk/part_efi.c:907:4: warning: format '%zX' expects argument of
>>  type 'size_t', but argument 3 has type 'long unsigned int'
>>  [-Wformat]
>> cmd/lzmadec.c: In function 'do_lzmadec':
>> cmd/lzmadec.c:39:12: warning: passing argument 2 of
>>  'lzmaBuffToBuffDecompress' from incompatible pointer type
>>  [enabled by default]
>> include/lzma/../../lib/lzma/LzmaTools.h:17:12: note: expected
>>  'SizeT *' but argument is of type 'long unsigned int *'
>> lib/hashtable.c: In function 'hexport_r':
>> lib/hashtable.c:605:2: warning: format '%zu' expects argument of
>>  type 'size_t', but argument 5 has type 'long unsigned int'
>>  [-Wformat]
>> lib/hashtable.c:661:5: warning: format '%zu' expects argument of
>>  type 'size_t', but argument 2 has type 'long unsigned int'
>>  [-Wformat]
>> lib/hashtable.c:661:5: warning: format '%zu' expects argument of
>>  type 'size_t', but argument 3 has type 'long unsigned int'
>>  [-Wformat]
>> lib/hashtable.c: In function 'himport_r':
>> lib/hashtable.c:793:3: warning: format '%zu' expects argument of
>>  type 'size_t', but argument 2 has type 'long unsigned int'
>>  [-Wformat]
>>
>> Signed-off-by: York Sun <york.sun at nxp.com>
>>
>> ---
>>
>> Changes in v4:
>>   New patch to fix compiling warnings for sandbox built on 32-bit host
>>   Still need to change CONFIG_SANDBOX_BITS_PER_LONG to 32 to avoid
>>   these warnings
>>   include/asm-generic/bitops/__fls.h:17:2: warning: left shift
>>    count >= width of type [enabled by default]
>>   include/asm-generic/bitops/__fls.h:19:3: warning: left shift
>>    count >= width of type [enabled by default]
>>   include/asm-generic/bitops/__fls.h:22:2: warning: left shift
>>    count >= width of type [enabled by default]
>>   include/asm-generic/bitops/__fls.h:26:2: warning: left shift
>>    count >= width of type [enabled by default]
>>   include/asm-generic/bitops/__fls.h:30:2: warning: left shift
>>    count >= width of type [enabled by default]
>>   include/asm-generic/bitops/__fls.h:34:2: warning: left shift
>>    count >= width of type [enabled by default]
>>   include/asm-generic/bitops/__fls.h:38:2: warning: left shift
>>    count >= width of type [enabled by default]
>>
>> Changes in v3: None
>> Changes in v2: None
>>
>>  arch/sandbox/cpu/cpu.c           |    3 ++-
>>  arch/sandbox/include/asm/types.h |    4 ++--
>>  arch/sandbox/lib/bootm.c         |    3 ++-
>>  arch/sandbox/lib/pci_io.c        |    2 +-
>>  cmd/bootm.c                      |    4 ++--
>>  cmd/lzmadec.c                    |    5 +++--
>>  disk/part_efi.c                  |    2 +-
>>  include/image.h                  |    6 ++++++
>>  lib/hashtable.c                  |    2 +-
>>  9 files changed, 20 insertions(+), 11 deletions(-)
> 
> I think this makes sense. But one nit - the % is not included in the
> macros in inttypes,h. What do you think about doing the same with your
> macro?

Sure. I can drop the % sign in the macro.

> 
>>
>> diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
>> index 196f3e1..7aad876 100644
>> --- a/arch/sandbox/cpu/cpu.c
>> +++ b/arch/sandbox/cpu/cpu.c
>> @@ -62,7 +62,8 @@ void *map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
>>         map_dev = NULL;
>>         if (enable_pci_map && !pci_map_physmem(paddr, &len, &map_dev, &ptr)) {
>>                 if (plen != len) {
>> -                       printf("%s: Warning: partial map at %x, wanted %lx, got %lx\n",
>> +                       printf("%s: Warning: partial map at " PRIpa
>> +                              ", wanted %lx, got %lx\n",
>>                                __func__, paddr, len, plen);
>>                 }
>>                 map_len = len;
>> diff --git a/arch/sandbox/include/asm/types.h b/arch/sandbox/include/asm/types.h
>> index 42c09e2..c3bb76e 100644
>> --- a/arch/sandbox/include/asm/types.h
>> +++ b/arch/sandbox/include/asm/types.h
>> @@ -53,8 +53,8 @@ typedef __UINT64_TYPE__ u64;
>>  #define BITS_PER_LONG  CONFIG_SANDBOX_BITS_PER_LONG
>>
>>  typedef unsigned long dma_addr_t;
>> -typedef u32 phys_addr_t;
>> -typedef u32 phys_size_t;
>> +typedef unsigned long phys_addr_t;
>> +typedef unsigned long phys_size_t;
>>
>>  #endif /* __KERNEL__ */
>>
>> diff --git a/arch/sandbox/lib/bootm.c b/arch/sandbox/lib/bootm.c
>> index d49c927..9ca89b5 100644
>> --- a/arch/sandbox/lib/bootm.c
>> +++ b/arch/sandbox/lib/bootm.c
>> @@ -54,7 +54,8 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
>>  {
>>         if (flag & (BOOTM_STATE_OS_GO | BOOTM_STATE_OS_FAKE_GO)) {
>>                 bootstage_mark(BOOTSTAGE_ID_RUN_OS);
>> -               printf("## Transferring control to Linux (at address %08lx)...\n",
>> +               printf("## Transferring control to Linux (at address " PRIpa
>> +                      ")...\n",
>>                        images->ep);
>>                 reset_cpu(0);
>>         }
>> diff --git a/arch/sandbox/lib/pci_io.c b/arch/sandbox/lib/pci_io.c
>> index 0de124f..3ada286 100644
>> --- a/arch/sandbox/lib/pci_io.c
>> +++ b/arch/sandbox/lib/pci_io.c
>> @@ -35,7 +35,7 @@ int pci_map_physmem(phys_addr_t paddr, unsigned long *lenp,
>>                 return 0;
>>         }
>>
>> -       debug("%s: failed: addr=%x\n", __func__, paddr);
>> +       debug("%s: failed: addr=" PRIpa "\n", __func__, paddr);
>>         return -ENOSYS;
>>  }
>>
>> diff --git a/cmd/bootm.c b/cmd/bootm.c
>> index 48738ac..5493c5d 100644
>> --- a/cmd/bootm.c
>> +++ b/cmd/bootm.c
>> @@ -566,8 +566,8 @@ static int bootz_start(cmd_tbl_t *cmdtp, int flag, int argc,
>>                                 load_addr);
>>         } else {
>>                 images->ep = simple_strtoul(argv[0], NULL, 16);
>> -               debug("*  kernel: cmdline image address = 0x%08lx\n",
>> -                       images->ep);
>> +               debug("*  kernel: cmdline image address = 0x" PRIpa "\n",
>> +                     images->ep);
>>         }
>>
>>         ret = bootz_setup(images->ep, &zi_start, &zi_end);
>> diff --git a/cmd/lzmadec.c b/cmd/lzmadec.c
>> index 1ad9ed6..8e370ef 100644
>> --- a/cmd/lzmadec.c
>> +++ b/cmd/lzmadec.c
>> @@ -20,7 +20,7 @@
>>  static int do_lzmadec(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
>>  {
>>         unsigned long src, dst;
>> -       unsigned long src_len = ~0UL, dst_len = ~0UL;
>> +       SizeT src_len = ~0UL, dst_len = ~0UL;
>>         int ret;
>>
>>         switch (argc) {
>> @@ -40,7 +40,8 @@ static int do_lzmadec(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
>>
>>         if (ret != SZ_OK)
>>                 return 1;
>> -       printf("Uncompressed size: %ld = 0x%lX\n", src_len, src_len);
>> +       printf("Uncompressed size: %ld = 0x%lX\n",
>> +              (ulong)src_len, (ulong)src_len);
>>         setenv_hex("filesize", src_len);
>>
>>         return 0;
>> diff --git a/disk/part_efi.c b/disk/part_efi.c
>> index e1b58c5..a59527b 100644
>> --- a/disk/part_efi.c
>> +++ b/disk/part_efi.c
>> @@ -10,7 +10,6 @@
>>   *   when CONFIG_SYS_64BIT_LBA is not defined, lbaint_t is 32 bits; this
>>   *   limits the maximum size of addressable storage to < 2 Terra Bytes
>>   */
>> -#include <asm/unaligned.h>
>>  #include <common.h>
>>  #include <command.h>
>>  #include <ide.h>
>> @@ -19,6 +18,7 @@
>>  #include <memalign.h>
>>  #include <part_efi.h>
>>  #include <linux/ctype.h>
>> +#include <asm/unaligned.h>
> 
> What is this for?

Not really my concern. I simply rearrange the order of include. <common.h> needs
to be on top to activate some macros for other header files to use.

> 
>>
>>  DECLARE_GLOBAL_DATA_PTR;
>>
>> diff --git a/include/image.h b/include/image.h
>> index 299d6d2..d77f46c 100644
>> --- a/include/image.h
>> +++ b/include/image.h
>> @@ -38,6 +38,12 @@ struct lmb;
>>  #include <lmb.h>
>>  #include <asm/u-boot.h>
>>  #include <command.h>
>> +#include <linux/types.h>
>> +#if    defined(CONFIG_PHYS_64BIT) || (BITS_PER_LONG > 32)
>> +#define PRIpa "0x%08llx"
>> +#else
>> +#define PRIpa "0x%08lx"
> 
> Can you use "%#08lx" ?

Yes.

> 
>> +#endif
>>
>>  /* Take notice of the 'ignore' property for hashes */
>>  #define IMAGE_ENABLE_IGNORE    1
>> diff --git a/lib/hashtable.c b/lib/hashtable.c
>> index 02b4105..ddc5541 100644
>> --- a/lib/hashtable.c
>> +++ b/lib/hashtable.c
>> @@ -13,6 +13,7 @@
>>   * SPDX-License-Identifier:    LGPL-2.1+
>>   */
>>
>> +# include <common.h>
>>  #include <errno.h>
>>  #include <malloc.h>
>>
>> @@ -29,7 +30,6 @@
>>  #  endif
>>  # endif
>>  #else                          /* U-Boot build */
>> -# include <common.h>
>>  # include <linux/string.h>
>>  # include <linux/ctype.h>
>>  #endif
>> --
>> 1.7.9.5
>>
> 
> I guess it is safe to include <common.h> here...

Same thing. I moved <common.h> up. I will drop the white space in next version
though.

York



More information about the U-Boot mailing list