On Thu, Jun 27, 2024 at 09:28:04PM +0200, Heinrich Schuchardt wrote:
>
Hi Heinrich,
Thanks for your review.
My comments below.
Best regards,
Vincent.
>
> Am 27. Juni 2024 19:06:29 MESZ schrieb "Vincent Stehlé"
> <vincent.ste...@arm.com>:
> >The scan_part() function uses a struct uuid to store the little-endian
> >partition type GUID, but this structure should be used only to contain a
> >big-endian UUID. Use an efi_guid_t instead.
> >
> >Signed-off-by: Vincent Stehlé <vincent.ste...@arm.com>
> >Cc: Simon Glass <s...@chromium.org>
> >Cc: Tom Rini <tr...@konsulko.com>
> >---
> > boot/bootmeth_cros.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/boot/bootmeth_cros.c b/boot/bootmeth_cros.c
> >index f015f2e1c75..1f83c14aeab 100644
> >--- a/boot/bootmeth_cros.c
> >+++ b/boot/bootmeth_cros.c
> >@@ -148,7 +148,7 @@ static int scan_part(struct udevice *blk, int partnum,
> > {
> > struct blk_desc *desc = dev_get_uclass_plat(blk);
> > struct vb2_keyblock *hdr;
> >- struct uuid type;
> >+ efi_guid_t type;
>
> Does Chrome OS only support GPT partitioning?
>
> > ulong num_blks;
> > int ret;
> >
> >@@ -161,7 +161,7 @@ static int scan_part(struct udevice *blk, int partnum,
> >
> > /* Check for kernel partition type */
> > log_debug("part %x: type=%s\n", partnum, info->type_guid);
> >- if (uuid_str_to_bin(info->type_guid, (u8 *)&type, UUID_STR_FORMAT_GUID))
> >+ if (uuid_str_to_bin(info->type_guid, type.b, UUID_STR_FORMAT_GUID))
> > return log_msg_ret("typ", -EINVAL);
>
> struct disk_partition containing a string which is only needed in the CLI
> instead of the 16 byte GUID was a bad idea to start with. Shouldn't we
> replace it or add least add a GUID field instead of first converting to
> string and than back to GUID?
I had a quick look and it seems that converting all those UUIDs from strings to
binary would indeed impact many places; let's separate this longer-term effort
from this change if you agree.
>
> >
> > if (memcmp(&cros_kern_type, &type, sizeof(type)))
>
> You could use the guidcmp() macro here.
Thanks for the tip; I will send a v2 series.
>
> Best regards
>
> Heinrich
>
