Re: [PATCH v2 0/2] cryptodisk: Allows UUIDs to be compared in a dash-insensitive manner

[Mihai Moldovan]

* On 3/24/21 5:24 AM, Glenn Washburn wrote:
> Aside from the fact that this doesn't handle negative values of n, this
> would be fine.

Neither do the original version or grub_strncasecmp()... and if you *can* get n
to be negative, I'd be highly impressed!

Jokes aside, it's unsigned, so there's no way for it to become negative. It
should probably handle 0 == n in a different way like grub_strncasecmp() does,
but that's easily added.


> 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.

Agree.


> 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.

Yeah, that's pretty obvious by it using grub_strncasecmp() in the end, but my
arguments would be that we don't need two passes over the input strings instead
of just one and could avoid the copies entirely. (Yes, these arguments are
clearly over-optimizations.)

Also, semantically, UUID parsing *is* different from simple string parsing,
which is also better reflected there.


> 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.

I'm not in any way authoritative, just adding my thoughts. :)

I don't really care which version gets in, but I figured that since it might be
a while until this lands in grub proper, I could still offer some insights.



Mihai

OpenPGP_signature
Description: OpenPGP digital signature