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@gmail.com Cc: Sam Protsenko joe.skb7@gmail.com Signed-off-by: Tom Rini trini@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@gmail.com