[PATCH] [readdir] Return correct inode number of .. directory

[PATCH] [readdir] Return correct inode number of .. directory

[Richard Yao]

dir_emit_dotdot() currently passes parent_ino(file->f_path.dentry) to
dir_emit(). Passing a dentry to parent_ino() is wrong. This should have
been parent_ino(file->f_path.dentry->i_ino).

Signed-off-by: Richard Yao <ryao@gentoo.org>
---
 include/linux/fs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9818747..7495f2e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2717,7 +2717,7 @@ static inline bool dir_emit_dot(struct file *file, struct dir_context *ctx)
 static inline bool dir_emit_dotdot(struct file *file, struct dir_context *ctx)
 {
 	return ctx->actor(ctx, "..", 2, ctx->pos,
-			  parent_ino(file->f_path.dentry), DT_DIR) == 0;
+			  parent_ino(file->f_path.dentry->i_ino), DT_DIR) == 0;
 }
 static inline bool dir_emit_dots(struct file *file, struct dir_context *ctx)
 {
-- 
1.8.1.5

Re: [PATCH] [readdir] Return correct inode number of .. directory

[Richard Yao]

[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]

Ignore that patch. It went out prematurely and unfortunately, it is
wrong. I wrote it when I was debugging an issue where I was seeing the
inode number was not appear in getdents output at userland, but that was
caused by something else.

On 08/08/2013 02:42 PM, Richard Yao wrote:
> dir_emit_dotdot() currently passes parent_ino(file->f_path.dentry) to
> dir_emit(). Passing a dentry to parent_ino() is wrong. This should have
> been parent_ino(file->f_path.dentry->i_ino).
> 
> Signed-off-by: Richard Yao <ryao@gentoo.org>
> ---
>  include/linux/fs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 9818747..7495f2e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2717,7 +2717,7 @@ static inline bool dir_emit_dot(struct file *file, struct dir_context *ctx)
>  static inline bool dir_emit_dotdot(struct file *file, struct dir_context *ctx)
>  {
>  	return ctx->actor(ctx, "..", 2, ctx->pos,
> -			  parent_ino(file->f_path.dentry), DT_DIR) == 0;
> +			  parent_ino(file->f_path.dentry->i_ino), DT_DIR) == 0;
>  }
>  static inline bool dir_emit_dots(struct file *file, struct dir_context *ctx)
>  {
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

Re: [PATCH] [readdir] Return correct inode number of .. directory

[Al Viro]

On Thu, Aug 08, 2013 at 02:42:22PM -0400, Richard Yao wrote:
> dir_emit_dotdot() currently passes parent_ino(file->f_path.dentry) to
> dir_emit(). Passing a dentry to parent_ino() is wrong. This should have
> been parent_ino(file->f_path.dentry->i_ino).

What the hell?  For the benefit of people who can't be arsed to learn
how to use grep, parent_ino() is defined in include/linux/fs.h and
that definition is

static inline ino_t parent_ino(struct dentry *dentry)
{
	ino_t res;

	/*
	 * Don't strictly need d_lock here? If the parent ino could change
	 * then surely we'd have a deeper race in the caller?
	 */
	spin_lock(&dentry->d_lock);
	res = dentry->d_parent->d_inode->i_ino;
	spin_unlock(&dentry->d_lock);
	return res;
}

Mind explaining your "passing dentry to parent_ino() is wrong"?  What else
to pass there?  Incidentally, struct dentry has no field called 'i_ino',
so with that patch the whole thing won't compile, period.

Consider the patch NAKed.

Re: [PATCH] [readdir] Return correct inode number of .. directory

[Richard Yao]

[-- Attachment #1: Type: text/plain, Size: 1378 bytes --]

I did not sleep last night while I hacked on certain things. I should
have just stepped away from my keyboard for a break a few hours before I
wrote that, but regrettably, I did not.

I sent an email retracting it almost immediately after I sent it. My
apologies for the noise.

On 08/08/2013 03:08 PM, Al Viro wrote:
> On Thu, Aug 08, 2013 at 02:42:22PM -0400, Richard Yao wrote:
>> dir_emit_dotdot() currently passes parent_ino(file->f_path.dentry) to
>> dir_emit(). Passing a dentry to parent_ino() is wrong. This should have
>> been parent_ino(file->f_path.dentry->i_ino).
> 
> What the hell?  For the benefit of people who can't be arsed to learn
> how to use grep, parent_ino() is defined in include/linux/fs.h and
> that definition is
> 
> static inline ino_t parent_ino(struct dentry *dentry)
> {
> 	ino_t res;
> 
> 	/*
> 	 * Don't strictly need d_lock here? If the parent ino could change
> 	 * then surely we'd have a deeper race in the caller?
> 	 */
> 	spin_lock(&dentry->d_lock);
> 	res = dentry->d_parent->d_inode->i_ino;
> 	spin_unlock(&dentry->d_lock);
> 	return res;
> }
> 
> Mind explaining your "passing dentry to parent_ino() is wrong"?  What else
> to pass there?  Incidentally, struct dentry has no field called 'i_ino',
> so with that patch the whole thing won't compile, period.
> 
> Consider the patch NAKed.
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]