[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 04/16] include/grub/misc.h: Fix edge case in grub_uuidcase
From: |
Vitaly Kuzmichev |
Subject: |
Re: [PATCH v5 04/16] include/grub/misc.h: Fix edge case in grub_uuidcasecmp() |
Date: |
Thu, 31 Aug 2023 17:32:14 +0000 |
Hi Daniel,
On Thu, 2023-08-31 at 19:09 +0200, Daniel Kiper wrote:
> On Tue, Aug 22, 2023 at 11:39:12PM +0200, Vitaly Kuzmichev wrote:
> > Fix comparison of two identical UUID strings ending with dash '-'.
> >
> > In this case grub_uuidcasecmp() passes through the null terminators
> > and actual result depends on whatever garbage follows them.
> >
> > So break immediately when it reaches the end in any of the strings
> > after a dash character '-'.
> >
> > Signed-off-by: Vitaly Kuzmichev <vitaly.kuzmichev@rtsoft.de>
> > ---
> > include/grub/misc.h | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/grub/misc.h b/include/grub/misc.h
> > index 1b35a167f..12fade5de 100644
> > --- a/include/grub/misc.h
> > +++ b/include/grub/misc.h
> > @@ -264,7 +264,8 @@ grub_uuidcasecmp (const char *uuid1, const char
> > *uuid2, grub_size_t n)
> > while ('-' == *uuid2)
> > ++uuid2;
> >
> > - if (grub_tolower ((grub_uint8_t) *uuid1) != grub_tolower
> > ((grub_uint8_t) *uuid2))
> > + if (!*uuid1 || !*uuid2 ||
>
> I prefer
>
> if (*uuid1 != '\0' || *uuid2 != '\0' || ...
>
> ... if it is what you meant...
Thank you for review.
Actually, I meant equality to null terminator:
if (*uuid1 == '\0' || *uuid2 == '\0' || ...
It should exit the loop when it finds one.
In the same function we have also a loop with similar condition without
comparison operator as well:
while (*uuid1 && *uuid2 && --n)
And also:
while ('-' == *uuid2)
If I change my 'if' to the variant that you suggest, this would be a
mix of three different coding styles in the same function.
Should I fix them all to match your taste?
With Best Regards,
Vitaly.
>
> If you are OK with that change you get Reviewed-by: Daniel Kiper
> <daniel.kiper@oracle.com>...
>
> > + grub_tolower ((grub_uint8_t) *uuid1) != grub_tolower
> > ((grub_uint8_t) *uuid2))
> > break;
>
> Daniel
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v5 05/16] commands/search: Add support to search by partition PARTUUID, Vitaly Kuzmichev, 2023/08/22
[PATCH v5 10/16] commands/search: Add support to search by partition PARTLABEL, Vitaly Kuzmichev, 2023/08/22
[PATCH v5 13/16] fs/f2fs: Simplify to use grub_utf16_to_utf8_alloc(), Vitaly Kuzmichev, 2023/08/22
[PATCH v5 07/16] include/grub/charset.h: Add grub_utf16_{strlen, strnlen}(), Vitaly Kuzmichev, 2023/08/22
[PATCH v5 06/16] include/grub/charset.h: Enhance grub_utf16_to_utf8(), Vitaly Kuzmichev, 2023/08/22