On Mon, 11 Feb, at 03:22:21PM, Al Viro wrote:
> > +   /*
> > +    * If the string we're being asked to compare doesn't match
> > +    * the expected format return "no match".
> > +    */
> > +   if (!efivarfs_valid_name(str, len))
> > +           return 1;
> > +   if (!(q = strchr(name->name, '-')))
> > +           return 1;
> 
> No.  Why check that again, when we'd already called ->d_hash() on the
> incoming name *and* candidate dentry?  And buggered off on any potential
> errors.

Good point.

> > +
> > +   /* Find part 1, the variable name. */
> > +   guid = q - (const char *)name->name;
> 
> No need to do strchr() for that - you know that name passes
> efivarfs_valid_name(), so you know how far from the end will GUID part begin.

Right.

> > +   /* Case-sensitive compare for the variable name */
> > +   if (memcmp(str, name->name, guid))
> > +           return 1;
> > +
> > +   /* Case-insensitive compare for the GUID */
> > +   return strcasecmp(&name->name[guid], &str[guid]);
> > +}
> 
> > +static int efivarfs_d_hash(const struct dentry *dentry,
> > +                      const struct inode *inode, struct qstr *qstr)
> > +{
> Egads, man...
> [snip the horror with copying the name]
> 
>         unsigned long hash = init_name_hash();
>       const unsigned char *s = qstr->name;
>       int len = qstr->len;
> 
>       if (!efivarfs_valid_name(s, len))
>               return -EINVAL;
>         while (len-- > GUID_LEN)
>               hash = partial_name_hash(*s++, hash);
>       /* GUID is case-insensitive. */
>       while (len--)
>               hash = partial_name_hash(tolower(*s++), hash);
>         return end_name_hash(hash);

Oh, wow, yes that is much nicer. I didn't realise we could build the
hash incrementally like that. Very cool.

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to