[PATCH] image.h: Change android_image_get_dtb* to use uint and not u32

Eugeniu Rosca erosca at de.adit-jv.com
Sun Feb 16 17:23:14 CET 2020


Hi Tom,
Cc: Masahiro Yamada

On Fri, Feb 14, 2020 at 12:38:19PM -0500, Tom Rini wrote:
> The image.h header can be used fairly widely in U-Boot builds.  We
> cannot use u32 here as it may be used in cases where we don't have that
> typedef available and don't want to expose it either.  Use uint instead
> here.
> 
> Cc: Eugeniu Rosca <roscaeugeniu at gmail.com>
> Cc: Sam Protsenko <joe.skb7 at gmail.com>
> Signed-off-by: Tom Rini <trini at konsulko.com>
> ---
>  include/image.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/image.h b/include/image.h
> index b316d167d8d7..1dc3b48d8689 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -1425,9 +1425,9 @@ int android_image_get_ramdisk(const struct andr_img_hdr *hdr,
>  			      ulong *rd_data, ulong *rd_len);
>  int android_image_get_second(const struct andr_img_hdr *hdr,
>  			      ulong *second_data, ulong *second_len);
> -bool android_image_get_dtbo(ulong hdr_addr, ulong *addr, u32 *size);
> -bool android_image_get_dtb_by_index(ulong hdr_addr, u32 index, ulong *addr,
> -				    u32 *size);
> +bool android_image_get_dtbo(ulong hdr_addr, ulong *addr, uint *size);
> +bool android_image_get_dtb_by_index(ulong hdr_addr, uint index, ulong *addr,
> +				    uint *size);

While I think the change is harmless and brings some consistency and
visual comfort when reviewing the types employed in 'include/image.h',
I can hardly imagine a real-life breakage introduced by u32 in
'include/image.h'.

That's because the 'image.h' itself seems to ensure the definition
of u32 and friends, as can be understood from below test:

 --------------------- 8< --------------------------

1. Create a dummy C file which includes <image.h>

$ cat cmd/my-test.c                                
#define FORCE_BREAKAGE
#include <image.h>

2. Force a build-time error on including "int-ll64.h", which defines
u32 and friends for all architectures, as per commit 3747bdbb2bb83a
("arch: types.h: factor out fixed width typedefs to int-ll64.h")
(thanks Yamada-san):

$ git diff  -- include/asm-generic/int-ll64.h
diff --git a/include/asm-generic/int-ll64.h b/include/asm-generic/int-ll64.h
index 7451718a61cd..f90dc2826071 100644
--- a/include/asm-generic/int-ll64.h
+++ b/include/asm-generic/int-ll64.h
@@ -15,6 +15,10 @@
  * header files exported to user space
  */

+#ifdef FORCE_BREAKAGE
+#error "Included, thus providing (s,u){8,16,32,64}"
+#endif
+
 typedef __signed__ char __s8;
 typedef unsigned char __u8;

3. Compile the newly created C file for sandbox

$ make cmd/my-test.o
  CC      cmd/my-test.o
In file included from ./arch/sandbox/include/asm/types.h:9,
                 from include/linux/types.h:5,
                 from include/linux/string.h:4,
                 from include/compiler.h:126,
                 from include/image.h:18,
                 from cmd/my-test.c:3:
include/asm-generic/int-ll64.h:19:2: error: #error "Included, thus providing (s,u){8,16,32,64}"

 --------------------- 8< --------------------------

So, to summarize, I don't fully understand the motivation behind this
change, unless there is a long-term goal to keep image.h (and/or other
U-Boot headers) free of u32 and Co.

Just to contrast the frequency of 'uint' vs 'u32' in v5.5 Linux release:

$ git log --oneline -S"\bu32\b" --pickaxe-regex v5.4..v5.5 | wc -l
  1020
$ git log --oneline -S"\buint\b" --pickaxe-regex v5.4..v5.5 | wc -l
  35

Assuming you still want the change, since there is no regression:
Reviewed-by: Eugeniu Rosca <roscaeugeniu at gmail.com>

-- 
Best Regards
Eugeniu Rosca


More information about the U-Boot mailing list