On Mon, 22 Mar 2021 23:53:16 +0100
Mihai Moldovan <io...@ionic.de> wrote:
> * 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);
> }
Aside from the fact that this doesn't handle negative values of n, this
would be fine.
> 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.
I don't think its as big a deal as it sounds. There are just two extra
arrays on the stack, so not memory management issues. Partly I chose to
implement it the way I did by using grub_strncasecmp because its pretty
trivial to see just by looking at the patches that I'm not changing any
core logic. But I'm not wedded to my implementation, I just want this
functionality. If I get time, I'll make a new version taking this in to
account.
Glenn
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
