Al Viro <viro@xxxxxxxxxxxxxxxxxx> writes:
> ... and check the "name might be unstable" predicate
> the right way.
>
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> ---
> fs/libfs.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 748ac5923154..3ad1b1b7fed6 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -1789,7 +1789,7 @@ int generic_ci_d_compare(const struct dentry *dentry, unsigned int len,
> {
> const struct dentry *parent;
> const struct inode *dir;
> - char strbuf[DNAME_INLINE_LEN];
> + union shortname_store strbuf;
> struct qstr qstr;
>
> /*
> @@ -1809,22 +1809,23 @@ int generic_ci_d_compare(const struct dentry *dentry, unsigned int len,
> if (!dir || !IS_CASEFOLDED(dir))
> return 1;
>
> + qstr.len = len;
> + qstr.name = str;
> /*
> * If the dentry name is stored in-line, then it may be concurrently
> * modified by a rename. If this happens, the VFS will eventually retry
> * the lookup, so it doesn't matter what ->d_compare() returns.
> * However, it's unsafe to call utf8_strncasecmp() with an unstable
> * string. Therefore, we have to copy the name into a temporary buffer.
This part of the comment needs updating since there is no more copying.
> + * As above, len is guaranteed to match str, so the shortname case
> + * is exactly when str points to ->d_shortname.
> */
> - if (len <= DNAME_INLINE_LEN - 1) {
> - memcpy(strbuf, str, len);
> - strbuf[len] = 0;
> - str = strbuf;
> + if (qstr.name == dentry->d_shortname.string) {
> + strbuf = dentry->d_shortname; // NUL is guaranteed to be in there
> + qstr.name = strbuf.string;
> /* prevent compiler from optimizing out the temporary buffer */
> barrier();
If I read the code correctly, I admit I don't understand how this
guarantees the stability. Aren't you just assigning qstr.name back the
same value it had in case of an inlined name through a bounce pointer?
The previous implementation made sense to me, since the memcpy only
accessed each character once, and we guaranteed the terminating
character explicitly, but I'm having a hard time with this version.
> }
> - qstr.len = len;
> - qstr.name = str;
>
> return utf8_strncasecmp(dentry->d_sb->s_encoding, name, &qstr);
> }
--
Gabriel Krisman Bertazi
