Re: [PATCH 2/2] efivarfs: guid part of filenames are case-insensitive

[Al Viro <viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>]

On Mon, Feb 11, 2013 at 03:22:21PM +0000, Al Viro wrote:
> > +static int efivarfs_d_compare(const struct dentry *parent, const struct inode *pinode,
> > +			      const struct dentry *dentry, const struct inode *inode,
> > +			      unsigned int len, const char *str,
> > +			      const struct qstr *name)
> > +{
> > +	const char *q;
> > +	int guid;
> > +
> > +	/*
> > +	 * 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.
> 
> > +
> > +	/* 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.
> 
> > +	/* Case-sensitive compare for the variable name */
> > +	if (memcmp(str, name->name, guid))
> > +		return 1;

... and by the way, you need to compare lengths first, or that memcmp()
risks running out of mapped page.  Sure, it's NUL-terminated, but memcmp()
is *not* required to compare left-to-right; it's arch-dependent and the
very first memory access have every right to be at str + guid - 1.