[PATCH 2/6] boot: android: Add image_android_get_version()

[Igor Opaniuk]

Hi Mattijs,

On Thu, Jun 6, 2024 at 2:24 PM Mattijs Korpershoek
<mkorpershoek at baylibre.com> wrote:
>
> When reading a boot image header, we may need to retrieve the header
> version.
>
> Add a helper function for it.
>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
> ---
>  boot/image-android.c | 7 ++++++-
>  include/image.h      | 7 +++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/boot/image-android.c b/boot/image-android.c
> index ddd8ffd5e540..4f8fb51585eb 100644
> --- a/boot/image-android.c
> +++ b/boot/image-android.c
> @@ -185,7 +185,7 @@ bool android_image_get_data(const void *boot_hdr, const void *vendor_boot_hdr,
>                 return false;
>         }
>
> -       if (((struct andr_boot_img_hdr_v0 *)boot_hdr)->header_version > 2) {
> +       if (android_image_get_version(boot_hdr) > 2) {
>                 if (!vendor_boot_hdr) {
>                         printf("For boot header v3+ vendor boot image has to be provided\n");
>                         return false;
> @@ -203,6 +203,11 @@ bool android_image_get_data(const void *boot_hdr, const void *vendor_boot_hdr,
>         return true;
>  }
>
> +u32 android_image_get_version(const void *hdr)
> +{
> +       return ((struct andr_boot_img_hdr_v0 *)hdr)->header_version;
> +}
> +
>  static ulong android_image_get_kernel_addr(struct andr_image_data *img_data)
>  {
>         /*
> diff --git a/include/image.h b/include/image.h
> index acffd17e0dfd..18e5ced5ab42 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -1963,6 +1963,13 @@ bool is_android_boot_image_header(const void *hdr);
>   */
>  bool is_android_vendor_boot_image_header(const void *vendor_boot_img);
>
> +/**
> + * android_image_get_version() - Retrieve the boot.img version
> + *
> + * Return: Android boot image header version.
> + */
> +u32 android_image_get_version(const void *hdr);
> +
>  /**
>   * get_abootimg_addr() - Get Android boot image address
>   *
>
> --
> 2.45.0
>
why introduce a helper function if there is only one user of it?

android_image_get_data() expects andr_boot_img_hdr_v0 anyway,
as it has an explicit check for it in the very beginning
(is_android_boot_image_header()).

Have you considered adjusting android_image_get_data() declaration, and just use
andr_boot_img_hdr_v0 *boot_hdr as first param instead (like it's done
for example in
android_boot_image_v0_v1_v2_parse_hdr()) and then rely on implicit
cast when this
function is used.

this is of course all a matter of preference, just thinking out loud

-- 
Best regards - Atentamente - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk at gmail.com
skype: igor.opanyuk
https://www.linkedin.com/in/iopaniuk