[PATCH v5 6/6] common: android_ab: fix slot suffix for abc block

[Sam Protsenko]

On Thu, Oct 17, 2024 at 9:12 AM Dmitry Rokosov
<ddrokosov at salutedevices.com> wrote:
>
> To align with the official Android BCB (Bootloader Control Block)
> specifications, it's important to note that the slot_suffix should start
> with an underscore symbol.
>
> For a comprehensive understanding of the expected slot_suffix format in
> userspace, please refer to the provided reference [1].
>
> Links:
> [1] - https://source.android.com/docs/core/architecture/bootloader/updating#slots
>
> Based-on: https://android-review.googlesource.com/c/platform/external/u-boot/+/1446439
> Reviewed-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
> Reviewed-by: Simon Glass <sjg at chromium.org>
> Tested-by: Guillaume La Roque <glaroque at baylibre.com>
> Signed-off-by: Dmitry Rokosov <ddrokosov at salutedevices.com>

Would be nice to add "Fixes:" tag here, pointing to the corresponding
commit where the issue was introduced (see kernel docs for details).
It could be quite useful for possible stable branches and other
purposes, I'd recommend to add that tag for all fixes if you have more
in this series.

> ---
>  boot/android_ab.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/boot/android_ab.c b/boot/android_ab.c
> index c93e51541019d0fe793303c4b3d5286df061906f..a287eac04fe88ad08bdcf1b1b1d6e564d503d800 100644
> --- a/boot/android_ab.c
> +++ b/boot/android_ab.c
> @@ -52,7 +52,7 @@ static int ab_control_default(struct bootloader_control *abc)
>         if (!abc)
>                 return -EFAULT;
>
> -       memcpy(abc->slot_suffix, "a\0\0\0", 4);
> +       memcpy(abc->slot_suffix, "_a\0\0", 4);
>         abc->magic = BOOT_CTRL_MAGIC;
>         abc->version = BOOT_CTRL_VERSION;
>         abc->nb_slot = NUM_SLOTS;
> @@ -328,7 +328,8 @@ int ab_select_slot(struct blk_desc *dev_desc, struct disk_partition *part_info,
>                  * or the device tree.
>                  */
>                 memset(slot_suffix, 0, sizeof(slot_suffix));
> -               slot_suffix[0] = BOOT_SLOT_NAME(slot);
> +               slot_suffix[0] = '_';
> +               slot_suffix[1] = BOOT_SLOT_NAME(slot);

AFAIU, this changes the behavior of two above functions, and
consequently of "bcb ab_select" command? If so, just to double check:
were all users of those reworked correspondingly? I can see next
occurrences (there may be more):

$ grep -sIrHn '"_' boot/bootmeth_android.c

boot/bootmeth_android.c:74:    sprintf(partname, BOOT_PART_NAME "_%s",
priv->slot);
boot/bootmeth_android.c:111:    sprintf(partname,
VENDOR_BOOT_PART_NAME "_%s", priv->slot);
boot/bootmeth_android.c:156:    sprintf(slot_suffix, "_%s", priv->slot);
boot/bootmeth_android.c:397:    sprintf(slot_suffix, "_%s", priv->slot);

$ grep -sIrHn 'slot_suffix _' include/configs/
include/configs/ti_omap5_common.h:107:    "setenv slot_suffix _${slot_name};"
include/configs/meson64_android.h:65:        "setenv slot_suffix
_${current_slot}; " \

>                 if (memcmp(abc->slot_suffix, slot_suffix,
>                            sizeof(slot_suffix))) {
>                         memcpy(abc->slot_suffix, slot_suffix,
>
> --
> 2.43.0
>