[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 0/2] cryptodisk: Allows UUIDs to be compared in a dash-ins
From: |
Mihai Moldovan |
Subject: |
Re: [PATCH v2 0/2] cryptodisk: Allows UUIDs to be compared in a dash-insensitive manner |
Date: |
Mon, 22 Mar 2021 23:53:16 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 |
* On 3/21/21 9:06 PM, Glenn Washburn wrote:
> I have another version of grub_uuidcasecmp which is more efficient by not
> copying the UUID bytes to another buffer to strip out the dashes. [...]
At first, I didn't understand why you would need to copy the original UUIDs in
the first place, but then ...
> However, I'm not sure we're so memory constrained that the added complexity
> (its not trivial to verify correctness) is worth it here, so I've chosen to
> submit this simpler version.
I don't think that memory is so scarce that allocating two tiny string buffers
on the stack would be problematic, but ...
> If that assumption isn't valid, I can submit
> the more complex version. This current version uses an extra buffer copy in
> grub_uuidcasecmp to strip out dashes so that grub_strncasecmp can be called.
Do you do all of that just to be able to use grub_strncasecmp()?
Wouldn't it make more sense to just duplicate a bit of code in this case and do
away with all additional buffers and complexities?
Untested, written from the top of my head without any syntax check or the like:
static inline int
grub_uuidcasecmp (const char *uuid1, const char *uuid2, grub_size_t n)
{
const char *s1 = uuid1,
*s2 = uuid2;
/*
* Assume that n is the number of valid non-dash characters in a UUID
* and that both UUIDs are sized the same.
*/
for (grub_size_t i = 0; i < n; ++i)
{
/* Skip forward to non-dash on both UUIDs. */
while ('-' == *s1)
{
++s1;
}
while ('-' == *s2)
{
++s2;
}
if (grub_tolower (*s1) != grub_tolower (*s2))
{
break;
}
++s1;
++s2;
}
return (int) grub_tolower ((grub_uint8_t) *s1)
- (int) grub_tolower ((grub_uint8_t) *s2);
}
That is, admittedly, copying the actual part of grub_strncasecmp(), but we're
doing away with all copies and additional buffers and it's different enough to
warrant being a different function, I figure.
Mihai
OpenPGP_signature
Description: OpenPGP digital signature