Hi Igor,
Thank you for the review.
On lun., juin 10, 2024 at 11:20, Igor Opaniuk igor.opaniuk@gmail.com wrote:
Hi Mattijs,
On Thu, Jun 6, 2024 at 2:24 PM Mattijs Korpershoek mkorpershoek@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@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?
I added a second user of the helper function in patch 5/6.
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()).
Right.
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
I've given this some more thought, and since I'm already using struct andr_boot_img_hdr_v0 in bootmeth_android/scan_boot_part(), I will drop this patch for v2.
The helper seems indeed a bit useless given that we can use the struct's member for this.
Thanks!
-- Best regards - Atentamente - Meilleures salutations
Igor Opaniuk
mailto: igor.opaniuk@gmail.com skype: igor.opanyuk https://www.linkedin.com/in/iopaniuk