From: Linus Torvalds <torvalds@linux-foundation.org>
To: David Howells <dhowells@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
bfields@fieldses.org, hugh@veritas.com, hch@infradead.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] EXPORTFS: Don't return NULL from fh_to_dentry()/fh_to_parent() [ver #4]
Date: Fri, 5 Dec 2008 08:52:13 -0800 (PST) [thread overview]
Message-ID: <alpine.LFD.2.00.0812050848060.3386@nehalem.linux-foundation.org> (raw)
In-Reply-To: <20081205162318.20732.73054.stgit@warthog.procyon.org.uk>
On Fri, 5 Dec 2008, David Howells wrote:
>
> Returning NULL from fh_to_dentry() and fh_to_parent() is not permitted, so
> return -ESTALE instead. exportfs_decode_fh() does not check for NULL, but
> will try to dereference it as IS_ERR() does not check for it.
>
> The generic_fh_to_dentry() and generic_fh_to_parent() functions also no longer
> return NULL, but return -ESTALE instead.
I really don't like this one.
So just about every fh_to_dentry() returns NULL for some error case.
Instead of considering that a problem, maybe it should tell us something.
There is _one_ single caller of that function afaik, why shouldn't we just
teach the caller that dammit, lots of people want to just return NULL.
It's not like returning NULL doesn't make sense. Quite frankly, I think it
makes a lot more sense than returning -ESTALE, which is a very unnatural
error for most filesystems.
So quite frankly, I'd much rather just see the simpler patch. Have
exportfs handle the case.
Are there other callers?
Linus
---
fs/exportfs/expfs.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index 80246ba..e9b378c 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -369,6 +369,8 @@ struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
result = nop->fh_to_dentry(mnt->mnt_sb, fid, fh_len, fileid_type);
if (IS_ERR(result))
return result;
+ if (!result)
+ return ERR_PTR(-ESTALE);
if (S_ISDIR(result->d_inode->i_mode)) {
/*
next prev parent reply other threads:[~2008-12-05 16:53 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-12-05 16:23 David Howells
2008-12-05 16:39 ` Hugh Dickins
2008-12-05 16:52 ` Linus Torvalds [this message]
2008-12-05 17:42 ` J. Bruce Fields
2008-12-09 9:52 ` Christoph Hellwig
2008-12-05 18:49 ` David Howells
2008-12-05 19:02 ` Linus Torvalds
2008-12-08 23:24 ` [PATCH] EXPORTFS: handle NULL returns from fh_to_dentry()/fh_to_parent() J. Bruce Fields
2008-12-09 10:30 ` David Howells
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.LFD.2.00.0812050848060.3386@nehalem.linux-foundation.org \
--to=torvalds@linux-foundation.org \
--cc=akpm@linux-foundation.org \
--cc=bfields@fieldses.org \
--cc=dhowells@redhat.com \
--cc=hch@infradead.org \
--cc=hugh@veritas.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®