| Message ID | 20240314103404.2457718-1-keiichiw@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fuse: Do NULL check instead of IS_ERR in atomic_open | expand |
I realized this patch was wrong, as I misunderstood the spec of d_splice_alias. d_splice_alias can return NULL in a success case, so the original code was totally correct. Please ignore this patch. Sorry for the noise! Keiichi On Thu, Mar 14, 2024 at 7:34 PM Keiichi Watanabe <keiichiw@chromium.org> wrote: > > Since d_splice_alias returns NULL on error, we need to do NUL check > instead of IS_ERR. > > Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org> > --- > fs/fuse/dir.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c > index 4ae89f428243..4843a749dd91 100644 > --- a/fs/fuse/dir.c > +++ b/fs/fuse/dir.c > @@ -914,7 +914,7 @@ static int _fuse_atomic_open(struct inode *dir, struct dentry *entry, > alias = d_exact_alias(entry, inode); > if (!alias) { > alias = d_splice_alias(inode, entry); > - if (IS_ERR(alias)) { > + if (!alias) { > /* > * Close the file in user space, but do not unlink it, > * if it was created - with network file systems other > @@ -928,8 +928,7 @@ static int _fuse_atomic_open(struct inode *dir, struct dentry *entry, > } > } > > - if (alias) > - entry = alias; > + entry = alias; // alias must not be NULL here. > } > > fuse_change_entry_timeout(entry, &outentry); > -- > 2.44.0.291.gc1ea87d7ee-goog >
On Thu, Mar 14, 2024 at 07:34:04PM +0900, Keiichi Watanabe wrote: > Since d_splice_alias returns NULL on error, we need to do NUL check > instead of IS_ERR. d_splice_alias() does *NOT* return NULL on error. Never did. Moreover, passing it a pointer to non-directory inode will definitely return NULL. So will passing it a pointer to directory inode that currently has no aliases. NAK.
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 4ae89f428243..4843a749dd91 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -914,7 +914,7 @@ static int _fuse_atomic_open(struct inode *dir, struct dentry *entry, alias = d_exact_alias(entry, inode); if (!alias) { alias = d_splice_alias(inode, entry); - if (IS_ERR(alias)) { + if (!alias) { /* * Close the file in user space, but do not unlink it, * if it was created - with network file systems other @@ -928,8 +928,7 @@ static int _fuse_atomic_open(struct inode *dir, struct dentry *entry, } } - if (alias) - entry = alias; + entry = alias; // alias must not be NULL here. } fuse_change_entry_timeout(entry, &outentry);
Since d_splice_alias returns NULL on error, we need to do NUL check instead of IS_ERR. Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org> --- fs/fuse/dir.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)