[PATCH v4 2/2] android_ab: Fix ANDROID_AB_BACKUP_OFFSET

2024-03-12 Thread Colin McAllister
From: Colin McAllister 

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.

The code included by ANDROID_AB_BACKUP_OFFSET has been refactored to no
longer be conditionally compiled by preprocessor conditionals and
instead use C conditionals. This better aligns with the Linux kernel
style guide.

Fixes: 3430f24bc6 ("android_ab: Try backup booloader_message")
Signed-off-by: Colin McAllister 
Cc: Joshua Watt 
Cc: Simon Glass 
Signed-off-by: Colin McAllister 
---
v2:
  - Replaced #if conditionals with C if conditionals
  - Opted to use CONFIG_ANDROID_AB_BACKUP_OFFSET directly instead of
macros in kconfig.h as CONFIG_ANDROID_AB_BACKUP_OFFSET is not a
boolean or tristate value and doesn't have different values when
building SPL or TPL.
v3:
  - Added "Fixes:" tag
v4:
  - No changes

 boot/android_ab.c | 97 ++-
 1 file changed, 45 insertions(+), 52 deletions(-)

diff --git a/boot/android_ab.c b/boot/android_ab.c
index 9a3d15ec60..f547aa64e4 100644
--- a/boot/android_ab.c
+++ b/boot/android_ab.c
@@ -187,13 +187,12 @@ int ab_select_slot(struct blk_desc *dev_desc, struct 
disk_partition *part_info,
   bool dec_tries)
 {
struct bootloader_control *abc = NULL;
+   struct bootloader_control *backup_abc = NULL;
u32 crc32_le;
int slot, i, ret;
bool store_needed = false;
+   bool valid_backup = false;
char slot_suffix[4];
-#if ANDROID_AB_BACKUP_OFFSET
-   struct bootloader_control *backup_abc = NULL;
-#endif
 
ret = ab_control_create_from_disk(dev_desc, part_info, , 0);
if (ret < 0) {
@@ -205,53 +204,49 @@ int ab_select_slot(struct blk_desc *dev_desc, struct 
disk_partition *part_info,
return ret;
}
 
-#if ANDROID_AB_BACKUP_OFFSET
-   ret = ab_control_create_from_disk(dev_desc, part_info, _abc,
- ANDROID_AB_BACKUP_OFFSET);
-   if (ret < 0) {
-   free(abc);
-   return ret;
+   if (CONFIG_ANDROID_AB_BACKUP_OFFSET) {
+   ret = ab_control_create_from_disk(dev_desc, part_info, 
_abc,
+ 
CONFIG_ANDROID_AB_BACKUP_OFFSET);
+   if (ret < 0) {
+   free(abc);
+   return ret;
+   }
}
-#endif
 
crc32_le = ab_control_compute_crc(abc);
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
-   crc32_le = ab_control_compute_crc(backup_abc);
-   if (backup_abc->crc32_le != crc32_le) {
-   log_err("ANDROID: Invalid backup CRC-32 ");
-   log_err("expected %.8x, found %.8x),",
-   crc32_le, backup_abc->crc32_le);
-#endif
-
-   log_err("re-initializing A/B metadata.\n");
+   if (CONFIG_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 ");
+   log_err("(expected %.8x, found %.8x),",
+   crc32_le, backup_abc->crc32_le);
+   } else {
+   valid_backup = true;
+   log_info(" copying A/B metadata from 
backup.\n");
+   memcpy(abc, backup_abc, sizeof(*abc));
+   }
+   }
 
+   if (!valid_backup) {
+   log_err(" re-initializing A/B metadata.\n");
ret = ab_control_default(abc);
if (ret < 0) {
-#if ANDROID_AB_BACKUP_OFFSET
-   free(backup_abc);
-#endif
+   if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
+   free(backup_abc);
free(abc);
return -ENODATA;
}
-#if ANDROID_AB_BACKUP_OFFSET
-   } else {
-   /*
-* Backup is valid. Copy it to the primary
-*/
-   memcpy(abc, backup_abc, sizeof(*abc));
}
-#endif
store_needed = true;
}
 
if (abc->magic != BOOT_CTRL_MAGIC) {
  

[PATCH v4 1/2] android_ab: Add missing semicolon

2024-03-12 Thread Colin McAllister
From: Colin McAllister 

Found a missing semicolon in code protected by a #if that will never
evaluate to true due to a separate issue. Fixing this issue before
addressing the #if.

Fixes: 3430f24bc6 ("android_ab: Try backup booloader_message")
Signed-off-by: Colin McAllister 
Cc: Joshua Watt 
Cc: Simon Glass 
Signed-off-by: Colin McAllister 
---
v2: No changes
v3: Added "Fixes:" tag
v4: No changes

 boot/android_ab.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/boot/android_ab.c b/boot/android_ab.c
index c9df6d2b4b..9a3d15ec60 100644
--- a/boot/android_ab.c
+++ b/boot/android_ab.c
@@ -221,7 +221,7 @@ int ab_select_slot(struct blk_desc *dev_desc, struct 
disk_partition *part_info,
 #if 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 ")
+   log_err("ANDROID: Invalid backup CRC-32 ");
log_err("expected %.8x, found %.8x),",
crc32_le, backup_abc->crc32_le);
 #endif
-- 
2.34.1



[PATCH v4 0/2] Fix Android A/B backup

2024-03-12 Thread Colin McAllister
- Addresses compiler error due to missing semicolon
- Removes use of preprocessor macros with ANDROID_AB_BACKUP_OFFSET

Bug was found by noticing a semicolon was missing and not causing a
compiler error when CONFIG_ANDROID_AB_BACKUP_OFFSET was set. I submitted
a patch to fix the semicolon before fixing the #if's. Testing the latter
patch without the former with ANDORID_AB_BACKUP_OFFSET set will cause a
compiler error.

What's changed in V2?
-

The second verison of changes removes the #if preprocessor macros to use
C conditionals instead. There was some minor refactoring required to get
this to work, so I did more thourough testing to ensure the backup data
works as expected.

I also realized that CONFIG_VAL is not needed because there's no reason
for the SPL or TPL to have different values for the backup address. I
opted to just use CONFIG_ANDROID_AB_BACKUP_OFFSET directly.

What's changed in V3?
-

Added "Fixes:" tag to patches. Performed additonal testing as described
in the second paragraph in the testing notes below.

I opted to not use CONFIG_IS_ENABLED because that macro appears to only
be intended for y/n configurations. See note at top of linux/kconfig.h:

"Note that these only work with boolean and tristate options."

What's changed in V4?
-

Nothing other than sending the patch in through a different email
address in order to see if the Garmin SMTP server is borking the
patches.

How was this patch series tested?
-

I built for my target with CONFIG_ANDROID_AB_BACKUP_OFFSET set to
0x1000. I first verified that the device can normally boot. I then ran
dd before rebooting to corrupt the primary data. I confirmed that the
backup was used to properly restore the primary. I then corrupted both
the primary and backup data and confirmed that the primary and backup
were both reinitialized to default values. Lastly, I corrupted the
backup data and not the primary data and confirmed that the backup was
restored the primary data.

Addtionally, I disabled CONFIG_ANDROID_AB_BACKUP_OFFSET for my device
and confirmed that after I corrupt the primary data, no backup is used.
When the primary data is not corrupt, the device boots normally. This is
what I would expect, because CONFIG_ANDROID_AB_BACKUP_OFFSET's default
value is 0x0, which would evaluate to false for all C if conditions.

Colin McAllister (2):
  android_ab: Add missing semicolon
  android_ab: Fix ANDROID_AB_BACKUP_OFFSET

 boot/android_ab.c | 97 ++-
 1 file changed, 45 insertions(+), 52 deletions(-)

-- 
2.34.1



[PATCH v3 2/2] android_ab: Fix ANDROID_AB_BACKUP_OFFSET

2024-03-08 Thread Colin McAllister
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.

The code included by ANDROID_AB_BACKUP_OFFSET has been refactored to no
longer be conditionally compiled by preprocessor conditionals and
instead use C conditionals. This better aligns with the Linux kernel
style guide.

Fixes: 3430f24bc6 ("android_ab: Try backup booloader_message")
Signed-off-by: Colin McAllister 
Cc: Joshua Watt 
Cc: Simon Glass 
---
v2:
  - Replaced #if conditionals with C if conditionals
  - Opted to use CONFIG_ANDROID_AB_BACKUP_OFFSET directly instead of
macros in kconfig.h as CONFIG_ANDROID_AB_BACKUP_OFFSET is not a
boolean or tristate value and doesn't have different values when
building SPL or TPL.
v3:
  - Added "Fixes:" tag

 boot/android_ab.c | 97 ++-
 1 file changed, 45 insertions(+), 52 deletions(-)

diff --git a/boot/android_ab.c b/boot/android_ab.c
index 9a3d15ec60..f547aa64e4 100644
--- a/boot/android_ab.c
+++ b/boot/android_ab.c
@@ -187,13 +187,12 @@ int ab_select_slot(struct blk_desc *dev_desc, struct 
disk_partition *part_info,
   bool dec_tries)
 {
struct bootloader_control *abc = NULL;
+   struct bootloader_control *backup_abc = NULL;
u32 crc32_le;
int slot, i, ret;
bool store_needed = false;
+   bool valid_backup = false;
char slot_suffix[4];
-#if ANDROID_AB_BACKUP_OFFSET
-   struct bootloader_control *backup_abc = NULL;
-#endif

ret = ab_control_create_from_disk(dev_desc, part_info, , 0);
if (ret < 0) {
@@ -205,53 +204,49 @@ int ab_select_slot(struct blk_desc *dev_desc, struct 
disk_partition *part_info,
return ret;
}

-#if ANDROID_AB_BACKUP_OFFSET
-   ret = ab_control_create_from_disk(dev_desc, part_info, _abc,
- ANDROID_AB_BACKUP_OFFSET);
-   if (ret < 0) {
-   free(abc);
-   return ret;
+   if (CONFIG_ANDROID_AB_BACKUP_OFFSET) {
+   ret = ab_control_create_from_disk(dev_desc, part_info, 
_abc,
+ 
CONFIG_ANDROID_AB_BACKUP_OFFSET);
+   if (ret < 0) {
+   free(abc);
+   return ret;
+   }
}
-#endif

crc32_le = ab_control_compute_crc(abc);
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
-   crc32_le = ab_control_compute_crc(backup_abc);
-   if (backup_abc->crc32_le != crc32_le) {
-   log_err("ANDROID: Invalid backup CRC-32 ");
-   log_err("expected %.8x, found %.8x),",
-   crc32_le, backup_abc->crc32_le);
-#endif
-
-   log_err("re-initializing A/B metadata.\n");
+   if (CONFIG_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 ");
+   log_err("(expected %.8x, found %.8x),",
+   crc32_le, backup_abc->crc32_le);
+   } else {
+   valid_backup = true;
+   log_info(" copying A/B metadata from 
backup.\n");
+   memcpy(abc, backup_abc, sizeof(*abc));
+   }
+   }

+   if (!valid_backup) {
+   log_err(" re-initializing A/B metadata.\n");
ret = ab_control_default(abc);
if (ret < 0) {
-#if ANDROID_AB_BACKUP_OFFSET
-   free(backup_abc);
-#endif
+   if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
+   free(backup_abc);
free(abc);
return -ENODATA;
}
-#if ANDROID_AB_BACKUP_OFFSET
-   } else {
-   /*
-* Backup is valid. Copy it to the primary
-*/
-   memcpy(abc, backup_abc, sizeof(*abc));
}
-#endif
store_needed = true;
}

if (abc->magic != BOOT_CTRL_MAGIC) {
log_err("ANDROID: Unknown A/B metadata: %.8x\n", abc->magic);
-#if AND

[PATCH v3 1/2] android_ab: Add missing semicolon

2024-03-08 Thread Colin McAllister
Found a missing semicolon in code protected by a #if that will never
evaluate to true due to a separate issue. Fixing this issue before
addressing the #if.

Fixes: 3430f24bc6 ("android_ab: Try backup booloader_message")
Signed-off-by: Colin McAllister 
Cc: Joshua Watt 
Cc: Simon Glass 
---
v2: No changes
v3: Added "Fixes:" tag

 boot/android_ab.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/boot/android_ab.c b/boot/android_ab.c
index c9df6d2b4b..9a3d15ec60 100644
--- a/boot/android_ab.c
+++ b/boot/android_ab.c
@@ -221,7 +221,7 @@ int ab_select_slot(struct blk_desc *dev_desc, struct 
disk_partition *part_info,
 #if 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 ")
+   log_err("ANDROID: Invalid backup CRC-32 ");
log_err("expected %.8x, found %.8x),",
crc32_le, backup_abc->crc32_le);
 #endif
--
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.


[PATCH v3 0/2] Fix Android A/B backup

2024-03-08 Thread Colin McAllister
- Addresses compiler error due to missing semicolon
- Removes use of preprocessor macros with ANDROID_AB_BACKUP_OFFSET

Bug was found by noticing a semicolon was missing and not causing a
compiler error when CONFIG_ANDROID_AB_BACKUP_OFFSET was set. I submitted
a patch to fix the semicolon before fixing the #if's. Testing the latter
patch without the former with ANDORID_AB_BACKUP_OFFSET set will cause a
compiler error.

What's changed in V2?
-

The second verison of changes removes the #if preprocessor macros to use
C conditionals instead. There was some minor refactoring required to get
this to work, so I did more thourough testing to ensure the backup data
works as expected.

I also realized that CONFIG_VAL is not needed because there's no reason
for the SPL or TPL to have different values for the backup address. I
opted to just use CONFIG_ANDROID_AB_BACKUP_OFFSET directly.

What's changed in V3?
-

Added "Fixes:" tag to patches. Performed additonal testing as described
in the second paragraph in the testing notes below.

I opted to not use CONFIG_IS_ENABLED because that macro appears to only
be intended for y/n configurations. See note at top of linux/kconfig.h:

"Note that these only work with boolean and tristate options."

How was this patch series tested?
-

I built for my target with CONFIG_ANDROID_AB_BACKUP_OFFSET set to
0x1000. I first verified that the device can normally boot. I then ran
dd before rebooting to corrupt the primary data. I confirmed that the
backup was used to properly restore the primary. I then corrupted both
the primary and backup data and confirmed that the primary and backup
were both reinitialized to default values. Lastly, I corrupted the
backup data and not the primary data and confirmed that the backup was
restored the primary data.

Addtionally, I disabled CONFIG_ANDROID_AB_BACKUP_OFFSET for my device
and confirmed that after I corrupt the primary data, no backup is used.
When the primary data is not corrupt, the device boots normally. This is
what I would expect, because CONFIG_ANDROID_AB_BACKUP_OFFSET's default
value is 0x0, which would evaluate to false for all C if conditions.

Colin McAllister (2):
  android_ab: Add missing semicolon
  android_ab: Fix ANDROID_AB_BACKUP_OFFSET

 boot/android_ab.c | 97 ++-
 1 file changed, 45 insertions(+), 52 deletions(-)

--
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.


[PATCH v2 0/2] Fix Android A/B backup

2024-03-07 Thread Colin McAllister
- Addresses compiler error due to missing semicolon
- Removes use of preprocessor macros with ANDROID_AB_BACKUP_OFFSET

Bug was found by noticing a semicolon was missing and not causing a
compiler error when CONFIG_ANDROID_AB_BACKUP_OFFSET was set. I submitted
a patch to fix the semicolon before fixing the #if's. Testing the latter
patch without the former with ANDORID_AB_BACKUP_OFFSET set will cause a
compiler error.

What's changed in V2?
-

The second verison of changes removes the #if preprocessor macros to use
C conditionals instead. There was some minor refactoring required to get
this to work, so I did more thourough testing to ensure the backup data
works as expected.

I also realized that CONFIG_VAL is not needed because there's no reason
for the SPL or TPL to have different values for the backup address. I
opted to just use CONFIG_ANDROID_AB_BACKUP_OFFSET directly.

How was this patch series tested?
-

I built for my target with CONFIG_ANDROID_AB_BACKUP_OFFSET set to
0x1000. I first verified that the device can normally boot. I then ran
dd before rebooting to corrupt the primary data. I confirmed that the
backup was used to properly restore the primary. I then corrupted both
the primary and backup data and confirmed that the primary and backup
were both reinitialized to default values. Lastly, I corrupted the
backup data and not the primary data and confirmed that the backup was
restored the primary data.

Colin McAllister (2):
  android_ab: Add missing semicolon
  android_ab: Fix ANDROID_AB_BACKUP_OFFSET

 boot/android_ab.c | 97 ++-
 1 file changed, 45 insertions(+), 52 deletions(-)

--
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.


[PATCH v2 1/2] android_ab: Add missing semicolon

2024-03-07 Thread Colin McAllister
Found a missing semicolon in code protected by a #if that will never
evaluate to true due to a separate issue. Fixing this issue before
addressing the #if.

Signed-off-by: Colin McAllister 
Cc: Joshua Watt 
Cc: Simon Glass 
---
 boot/android_ab.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/boot/android_ab.c b/boot/android_ab.c
index c9df6d2b4b..9a3d15ec60 100644
--- a/boot/android_ab.c
+++ b/boot/android_ab.c
@@ -221,7 +221,7 @@ int ab_select_slot(struct blk_desc *dev_desc, struct 
disk_partition *part_info,
 #if 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 ")
+   log_err("ANDROID: Invalid backup CRC-32 ");
log_err("expected %.8x, found %.8x),",
crc32_le, backup_abc->crc32_le);
 #endif
--
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.


[PATCH v2 2/2] android_ab: Fix ANDROID_AB_BACKUP_OFFSET

2024-03-07 Thread Colin McAllister
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.

The code included by ANDROID_AB_BACKUP_OFFSET has been refactored to no
longer be conditionally compiled by preprocessor conditionals and
instead use C conditionals. This better aligns with the Linux kernel
style guide.

Signed-off-by: Colin McAllister 
Cc: Joshua Watt 
Cc: Simon Glass 
---
 boot/android_ab.c | 97 ++-
 1 file changed, 45 insertions(+), 52 deletions(-)

diff --git a/boot/android_ab.c b/boot/android_ab.c
index 9a3d15ec60..f547aa64e4 100644
--- a/boot/android_ab.c
+++ b/boot/android_ab.c
@@ -187,13 +187,12 @@ int ab_select_slot(struct blk_desc *dev_desc, struct 
disk_partition *part_info,
   bool dec_tries)
 {
struct bootloader_control *abc = NULL;
+   struct bootloader_control *backup_abc = NULL;
u32 crc32_le;
int slot, i, ret;
bool store_needed = false;
+   bool valid_backup = false;
char slot_suffix[4];
-#if ANDROID_AB_BACKUP_OFFSET
-   struct bootloader_control *backup_abc = NULL;
-#endif

ret = ab_control_create_from_disk(dev_desc, part_info, , 0);
if (ret < 0) {
@@ -205,53 +204,49 @@ int ab_select_slot(struct blk_desc *dev_desc, struct 
disk_partition *part_info,
return ret;
}

-#if ANDROID_AB_BACKUP_OFFSET
-   ret = ab_control_create_from_disk(dev_desc, part_info, _abc,
- ANDROID_AB_BACKUP_OFFSET);
-   if (ret < 0) {
-   free(abc);
-   return ret;
+   if (CONFIG_ANDROID_AB_BACKUP_OFFSET) {
+   ret = ab_control_create_from_disk(dev_desc, part_info, 
_abc,
+ 
CONFIG_ANDROID_AB_BACKUP_OFFSET);
+   if (ret < 0) {
+   free(abc);
+   return ret;
+   }
}
-#endif

crc32_le = ab_control_compute_crc(abc);
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
-   crc32_le = ab_control_compute_crc(backup_abc);
-   if (backup_abc->crc32_le != crc32_le) {
-   log_err("ANDROID: Invalid backup CRC-32 ");
-   log_err("expected %.8x, found %.8x),",
-   crc32_le, backup_abc->crc32_le);
-#endif
-
-   log_err("re-initializing A/B metadata.\n");
+   if (CONFIG_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 ");
+   log_err("(expected %.8x, found %.8x),",
+   crc32_le, backup_abc->crc32_le);
+   } else {
+   valid_backup = true;
+   log_info(" copying A/B metadata from 
backup.\n");
+   memcpy(abc, backup_abc, sizeof(*abc));
+   }
+   }

+   if (!valid_backup) {
+   log_err(" re-initializing A/B metadata.\n");
ret = ab_control_default(abc);
if (ret < 0) {
-#if ANDROID_AB_BACKUP_OFFSET
-   free(backup_abc);
-#endif
+   if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
+   free(backup_abc);
free(abc);
return -ENODATA;
}
-#if ANDROID_AB_BACKUP_OFFSET
-   } else {
-   /*
-* Backup is valid. Copy it to the primary
-*/
-   memcpy(abc, backup_abc, sizeof(*abc));
}
-#endif
store_needed = true;
}

if (abc->magic != BOOT_CTRL_MAGIC) {
log_err("ANDROID: Unknown A/B metadata: %.8x\n", abc->magic);
-#if ANDROID_AB_BACKUP_OFFSET
-   free(backup_abc);
-#endif
+   if (CONFIG_ANDROID_AB_BACKUP_OFFSET)
+   free(backup_abc);
free(abc);
return -ENODATA;
}
@@ -259,9 +254,8 @@ int ab_select_slot(struct blk_desc *dev_desc, struct 
disk_partition *part_info,
if (abc->version > BOOT_CTRL_VERSION) {
log_err("ANDRO

[PATCH 2/2] android_ab: Fix ANDROID_AB_BACKUP_OFFSET

2024-03-07 Thread Colin McAllister
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 
Cc: Joshua Watt 
Cc: Simon Glass 
---
 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, _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);
--
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,

[PATCH 1/2] android_ab: Add missing semicolon

2024-03-07 Thread Colin McAllister
Found a missing semicolon in code protected by a #if that will never
evaluate to true due to a separate issue. Fixing this issue before
addressing the #if.

Signed-off-by: Colin McAllister 
Cc: Joshua Watt 
Cc: Simon Glass 
---
 boot/android_ab.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/boot/android_ab.c b/boot/android_ab.c
index c9df6d2b4b..9a3d15ec60 100644
--- a/boot/android_ab.c
+++ b/boot/android_ab.c
@@ -221,7 +221,7 @@ int ab_select_slot(struct blk_desc *dev_desc, struct 
disk_partition *part_info,
 #if 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 ")
+   log_err("ANDROID: Invalid backup CRC-32 ");
log_err("expected %.8x, found %.8x),",
crc32_le, backup_abc->crc32_le);
 #endif
--
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.


[PATCH 0/2] Fix Android A/B backup

2024-03-07 Thread Colin McAllister
- Addresses compiler error due to missing semicolon
- Fixes use of ANDROID_AB_BACKUP_OFFSET in preprocessor macros

Bug was found by noticing a semicolon was missing and not causing a
compiler error when CONFIG_ANDROID_AB_BACKUP_OFFSET was set. I submitted
a patch to fix the semicolon before fixing the #if's. Testing the latter
patch without the former with ANDORID_AB_BACKUP_OFFSET set will cause a
compiler error.

Colin McAllister (2):
  android_ab: Add missing semicolon
  android_ab: Fix ANDROID_AB_BACKUP_OFFSET

 boot/android_ab.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

--
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.