grub-devel
[Top][All Lists]
Advanced

[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

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]