| Message ID | 20200217102353.11174-1-erosca@de.adit-jv.com |
|---|---|
| State | New |
| Headers | show |
| Series | image.h: use uint32_t instead of u32 in android_image_get_dtb* | expand |
On Mon, Feb 17, 2020 at 7:24 PM Eugeniu Rosca <erosca at de.adit-jv.com> wrote: > > Replace 'u32' by 'uint32_t' in image.h, since the former may lead to > build failures in U-Boot tooling (see [1]). > > Avoid using 'uint', since it is not a fixed-width type [2], potentially > leading to a dangerous mismatch between the prototypes and definitions > of the android_image_get_dtb* functions. > > This should be the quickest way to overcome the tooling build failure, > with more future-proof solutions being proposed by Yamada-san in [1]. > > [1] https://patchwork.ozlabs.org/patch/1238245/ > [2] Excerpt from https://en.cppreference.com/w/cpp/language/types > -----------8<------------ > Type specifier Width in bits by data model > LP32 ILP32 LLP64 LP64 > unsigned int 16 32 32 32 > -----------8<------------ > > Cc: Tom Rini <trini at konsulko.com> > Cc: Sam Protsenko <joe.skb7 at gmail.com> > Fixes: 7f2531502c74c0 ("image: android: Add routine to get dtbo params") > Fixes: c3bfad825a71ea ("image: android: Add functions for handling dtb field") > Suggested-by: Masahiro Yamada <masahiroy at kernel.org> > Signed-off-by: Eugeniu Rosca <erosca at de.adit-jv.com> > --- Reviewed-by: Masahiro Yamada <masahiroy at kernel.org>
Hi Eugeniu, On Mon, Feb 17, 2020 at 12:24 PM Eugeniu Rosca <erosca at de.adit-jv.com> wrote: > > Replace 'u32' by 'uint32_t' in image.h, since the former may lead to > build failures in U-Boot tooling (see [1]). > > Avoid using 'uint', since it is not a fixed-width type [2], potentially > leading to a dangerous mismatch between the prototypes and definitions > of the android_image_get_dtb* functions. > > This should be the quickest way to overcome the tooling build failure, > with more future-proof solutions being proposed by Yamada-san in [1]. > > [1] https://patchwork.ozlabs.org/patch/1238245/ > [2] Excerpt from https://en.cppreference.com/w/cpp/language/types > -----------8<------------ > Type specifier Width in bits by data model > LP32 ILP32 LLP64 LP64 > unsigned int 16 32 32 32 > -----------8<------------ > > Cc: Tom Rini <trini at konsulko.com> > Cc: Sam Protsenko <joe.skb7 at gmail.com> > Fixes: 7f2531502c74c0 ("image: android: Add routine to get dtbo params") > Fixes: c3bfad825a71ea ("image: android: Add functions for handling dtb field") > Suggested-by: Masahiro Yamada <masahiroy at kernel.org> > Signed-off-by: Eugeniu Rosca <erosca at de.adit-jv.com> > --- Reviewed-by: Sam Protsenko <joe.skb7 at gmail.com> > include/image.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/image.h b/include/image.h > index b316d167d8d7..1341fbed62ba 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, uint32_t *size); > +bool android_image_get_dtb_by_index(ulong hdr_addr, uint32_t index, ulong *addr, > + uint32_t *size); > ulong android_image_get_end(const struct andr_img_hdr *hdr); > ulong android_image_get_kload(const struct andr_img_hdr *hdr); > ulong android_image_get_kcomp(const struct andr_img_hdr *hdr); > -- > 2.25.0 >
On 2/20/20 12:45 PM, Sam Protsenko wrote: > Hi Eugeniu, > > On Mon, Feb 17, 2020 at 12:24 PM Eugeniu Rosca <erosca at de.adit-jv.com> wrote: >> >> Replace 'u32' by 'uint32_t' in image.h, since the former may lead to >> build failures in U-Boot tooling (see [1]). >> >> Avoid using 'uint', since it is not a fixed-width type [2], potentially >> leading to a dangerous mismatch between the prototypes and definitions >> of the android_image_get_dtb* functions. >> >> This should be the quickest way to overcome the tooling build failure, >> with more future-proof solutions being proposed by Yamada-san in [1]. >> >> [1] https://patchwork.ozlabs.org/patch/1238245/ >> [2] Excerpt from https://en.cppreference.com/w/cpp/language/types >> -----------8<------------ >> Type specifier Width in bits by data model >> LP32 ILP32 LLP64 LP64 >> unsigned int 16 32 32 32 >> -----------8<------------ >> >> Cc: Tom Rini <trini at konsulko.com> >> Cc: Sam Protsenko <joe.skb7 at gmail.com> >> Fixes: 7f2531502c74c0 ("image: android: Add routine to get dtbo params") >> Fixes: c3bfad825a71ea ("image: android: Add functions for handling dtb field") >> Suggested-by: Masahiro Yamada <masahiroy at kernel.org> >> Signed-off-by: Eugeniu Rosca <erosca at de.adit-jv.com> >> --- > > Reviewed-by: Sam Protsenko <joe.skb7 at gmail.com> uint32_t is defined in glibc's stdint.h and in our include/linux/types.h (which is included in common.h). We use image.h in tools/*.c in files which neither include common.h nor stdint.h. But for USE_HOSTCC=y include/compiler.h includes stdint.h. BUT changing a function definition in a header and not doing the same in the C-file looks quite inconsistent to me. Please, use the same types in both the C-file and in the header. Best regards Heinrich
To facilitate tracking, this patch has been superseded by:
https://patchwork.ozlabs.org/patch/1250963/
("image.h: isolate android_image_* functions from tooling")
diff --git a/include/image.h b/include/image.h index b316d167d8d7..1341fbed62ba 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, uint32_t *size); +bool android_image_get_dtb_by_index(ulong hdr_addr, uint32_t index, ulong *addr, + uint32_t *size); ulong android_image_get_end(const struct andr_img_hdr *hdr); ulong android_image_get_kload(const struct andr_img_hdr *hdr); ulong android_image_get_kcomp(const struct andr_img_hdr *hdr);
Replace 'u32' by 'uint32_t' in image.h, since the former may lead to build failures in U-Boot tooling (see [1]). Avoid using 'uint', since it is not a fixed-width type [2], potentially leading to a dangerous mismatch between the prototypes and definitions of the android_image_get_dtb* functions. This should be the quickest way to overcome the tooling build failure, with more future-proof solutions being proposed by Yamada-san in [1]. [1] https://patchwork.ozlabs.org/patch/1238245/ [2] Excerpt from https://en.cppreference.com/w/cpp/language/types -----------8<------------ Type specifier Width in bits by data model LP32 ILP32 LLP64 LP64 unsigned int 16 32 32 32 -----------8<------------ Cc: Tom Rini <trini at konsulko.com> Cc: Sam Protsenko <joe.skb7 at gmail.com> Fixes: 7f2531502c74c0 ("image: android: Add routine to get dtbo params") Fixes: c3bfad825a71ea ("image: android: Add functions for handling dtb field") Suggested-by: Masahiro Yamada <masahiroy at kernel.org> Signed-off-by: Eugeniu Rosca <erosca at de.adit-jv.com> --- include/image.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)