Hello Colin,
+CC Mattijs, Sam
On Thu, Mar 7, 2024 at 5:19 PM Colin McAllister colin.mcallister@garmin.com wrote:
Currently, setting CONFIG_AB_BACKUP_OFFSET in a target's defconfig will not actually enable the #if protected code in android_ab.c. This is because "CONFIG_" should have been prepended to the config macro, or the macros defined in kconfig.h could have been used.
Each use of AB_BACKUP_OFFSET is now wrapped within CONFIG_VAL().
Signed-off-by: Colin McAllister colin.mcallister@garmin.com Cc: Joshua Watt JPEWhacker@gmail.com Cc: Simon Glass sjg@chromium.org
boot/android_ab.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/boot/android_ab.c b/boot/android_ab.c index 9a3d15ec60..a80040749d 100644 --- a/boot/android_ab.c +++ b/boot/android_ab.c @@ -191,7 +191,7 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, int slot, i, ret; bool store_needed = false; char slot_suffix[4]; -#if ANDROID_AB_BACKUP_OFFSET +#if CONFIG_VAL(ANDROID_AB_BACKUP_OFFSET) struct bootloader_control *backup_abc = NULL; #endif
@@ -205,9 +205,9 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, return ret; }
-#if ANDROID_AB_BACKUP_OFFSET +#if CONFIG_VAL(ANDROID_AB_BACKUP_OFFSET) ret = ab_control_create_from_disk(dev_desc, part_info, &backup_abc,
ANDROID_AB_BACKUP_OFFSET);
CONFIG_VAL(ANDROID_AB_BACKUP_OFFSET)); if (ret < 0) { free(abc); return ret; @@ -218,7 +218,7 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, if (abc->crc32_le != crc32_le) { log_err("ANDROID: Invalid CRC-32 (expected %.8x, found %.8x),", crc32_le, abc->crc32_le); -#if ANDROID_AB_BACKUP_OFFSET +#if CONFIG_VAL(ANDROID_AB_BACKUP_OFFSET) crc32_le = ab_control_compute_crc(backup_abc); if (backup_abc->crc32_le != crc32_le) { log_err("ANDROID: Invalid backup CRC-32 "); @@ -230,13 +230,13 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info,
ret = ab_control_default(abc); if (ret < 0) {-#if ANDROID_AB_BACKUP_OFFSET +#if CONFIG_VAL(ANDROID_AB_BACKUP_OFFSET) free(backup_abc); #endif free(abc); return -ENODATA; } -#if ANDROID_AB_BACKUP_OFFSET +#if CONFIG_VAL(ANDROID_AB_BACKUP_OFFSET) } else { /* * Backup is valid. Copy it to the primary @@ -249,7 +249,7 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info,
if (abc->magic != BOOT_CTRL_MAGIC) { log_err("ANDROID: Unknown A/B metadata: %.8x\n",abc->magic); -#if ANDROID_AB_BACKUP_OFFSET +#if CONFIG_VAL(ANDROID_AB_BACKUP_OFFSET) free(backup_abc); #endif free(abc); @@ -259,7 +259,7 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, if (abc->version > BOOT_CTRL_VERSION) { log_err("ANDROID: Unsupported A/B metadata version: %.8x\n", abc->version); -#if ANDROID_AB_BACKUP_OFFSET +#if CONFIG_VAL(ANDROID_AB_BACKUP_OFFSET) free(backup_abc); #endif free(abc); @@ -338,7 +338,7 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, abc->crc32_le = ab_control_compute_crc(abc); ret = ab_control_store(dev_desc, part_info, abc, 0); if (ret < 0) { -#if ANDROID_AB_BACKUP_OFFSET +#if CONFIG_VAL(ANDROID_AB_BACKUP_OFFSET) free(backup_abc); #endif free(abc); @@ -346,14 +346,14 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info, } }
-#if ANDROID_AB_BACKUP_OFFSET +#if CONFIG_VAL(ANDROID_AB_BACKUP_OFFSET) /* * If the backup doesn't match the primary, write the primary * to the backup offset */ if (memcmp(backup_abc, abc, sizeof(*abc)) != 0) { ret = ab_control_store(dev_desc, part_info, abc,
ANDROID_AB_BACKUP_OFFSET);
CONFIG_VAL(ANDROID_AB_BACKUP_OFFSET)); if (ret < 0) { free(backup_abc); free(abc);
Thanks for the patch. The only suggestion: I see no reason in wrapping it in preprocessor conditionals (in this particular case), better to use C conditionals instead (with CONFIG_VAL macro) as explained in [1].
-
2.43.2
CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.
[1] https://www.kernel.org/doc/html/latest/process/coding-style.html#conditional...