| Message ID | 1347762073.13258.188.camel@deadeye.wl.decadent.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
I had a couple friends over today and we made a trivial patch to remove trailing slashes. We do not know C and have never created a patch for the kernel before, so there is undoubtedly a better way to do it. However we hope this helps in your efforts. This patch was created from the offending commit (c7f404b40a366). But I've also applied it to to Linus Torvalds' master HEAD (3f0c3c8fe30c7) with success. -Chris
On Sun, 2012-09-16 at 03:24 -0700, Chris Hiestand wrote: > I had a couple friends over today and we made a trivial patch to > remove trailing slashes. We do not know C and have never created a > patch for the kernel before, so there is undoubtedly a better way to > do it. However we hope this helps in your efforts. > > This patch was created from the offending commit (c7f404b40a366). But > I've also applied it to to Linus Torvalds' master HEAD (3f0c3c8fe30c7) > with success. This was my first thought - but what if userland provides a device name with a slash on the end? I think we have to report it back with the slash in that case. Ben.
On Sep 16, 2012, at 7:00 AM, Ben Hutchings wrote: > This was my first thought - but what if userland provides a device name > with a slash on the end? I think we have to report it back with the > slash in that case. That is a great point. We could not find any standard as to whether or not there should be a trailing slash. And from all the examples I could find, there seems to be a convention of omitting a trailing slash. However, if userland provides a trailing slash, it would seem appropriate to retain it. As a point of comparison, matching the given input is the behavior of 2.6.32-5 in Debian Squeeze. So I think your approach is better. -Chris
Hi Chris, Chris Hiestand wrote: > On Sep 16, 2012, at 7:00 AM, Ben Hutchings wrote: >> This was my first thought - but what if userland provides a device name >> with a slash on the end? I think we have to report it back with the >> slash in that case. [...] > As a point of comparison, matching the given input is the behavior of 2.6.32-5 in Debian Squeeze. > So I think your approach is better. Thanks for looking it over. Did you get a chance to test Ben's patch? Curious, Jonathan -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
I haven't had a chance to test it yet thanks. I'll be too busy until at least mid-next week, but I will test it then if nobody beats me to it. -Chris On Sep 30, 2012, at 2:23 PM, Jonathan Nieder <jrnieder@gmail.com> wrote: > Thanks for looking it over. Did you get a chance to test Ben's patch? > > Curious, > Jonathan
I've applied the patch to Linus' master HEAD (43c422eda99b894f18d1cca17bcd2401efaf7bd0, at the time) and the patch seems to work fine. /proc/self/mounts correctly reflects whether or not the user specified to use a trailing slash - and nothing is obviously broken. If the patch looks fine to others, I propose we move this patch into wherever is the next step for testing it more broadly: e.g. Debian experimental, and wherever it needs to go to end up in Linus' master branch. Thanks, Chris
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h index d554438..c38224a 100644 --- a/fs/nfs/internal.h +++ b/fs/nfs/internal.h @@ -354,7 +354,7 @@ extern void nfs_sb_deactive(struct super_block *sb); /* namespace.c */ extern char *nfs_path(char **p, struct dentry *dentry, - char *buffer, ssize_t buflen); + char *buffer, ssize_t buflen, bool canonical); extern struct vfsmount *nfs_d_automount(struct path *path); struct vfsmount *nfs_submount(struct nfs_server *, struct dentry *, struct nfs_fh *, struct nfs_fattr *); @@ -491,7 +491,7 @@ static inline char *nfs_devname(struct dentry *dentry, char *buffer, ssize_t buflen) { char *dummy; - return nfs_path(&dummy, dentry, buffer, buflen); + return nfs_path(&dummy, dentry, buffer, buflen, true); } /* diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c index 6559253..059975e 100644 --- a/fs/nfs/namespace.c +++ b/fs/nfs/namespace.c @@ -33,6 +33,8 @@ int nfs_mountpoint_expiry_timeout = 500 * HZ; * @dentry - pointer to dentry * @buffer - result buffer * @buflen - length of buffer + * @canonical - ensure there is exactly one slash after the original + * device (export) name; if false, return it verbatim * * Helper function for constructing the server pathname * by arbitrary hashed dentry. @@ -41,7 +43,8 @@ int nfs_mountpoint_expiry_timeout = 500 * HZ; * server side when automounting on top of an existing partition * and in generating /proc/mounts and friends. */ -char *nfs_path(char **p, struct dentry *dentry, char *buffer, ssize_t buflen) +char *nfs_path(char **p, struct dentry *dentry, char *buffer, ssize_t buflen, + bool canonical) { char *end; int namelen; @@ -74,7 +77,7 @@ rename_retry: rcu_read_unlock(); goto rename_retry; } - if (*end != '/') { + if (canonical && *end != '/') { if (--buflen < 0) { spin_unlock(&dentry->d_lock); rcu_read_unlock(); @@ -91,9 +94,11 @@ rename_retry: return end; } namelen = strlen(base); - /* Strip off excess slashes in base string */ - while (namelen > 0 && base[namelen - 1] == '/') - namelen--; + if (canonical) { + /* Strip off excess slashes in base string */ + while (namelen > 0 && base[namelen - 1] == '/') + namelen--; + } buflen -= namelen; if (buflen < 0) { spin_unlock(&dentry->d_lock); diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c index 017b4b0..94e8652 100644 --- a/fs/nfs/nfs4namespace.c +++ b/fs/nfs/nfs4namespace.c @@ -81,7 +81,7 @@ static char *nfs_path_component(const char *nfspath, const char *end) static char *nfs4_path(struct dentry *dentry, char *buffer, ssize_t buflen) { char *limit; - char *path = nfs_path(&limit, dentry, buffer, buflen); + char *path = nfs_path(&limit, dentry, buffer, buflen, true); if (!IS_ERR(path)) { char *path_component = nfs_path_component(path, limit); if (path_component) diff --git a/fs/nfs/super.c b/fs/nfs/super.c index d2c7f5d..630a1e2 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -765,7 +765,7 @@ int nfs_show_devname(struct seq_file *m, struct dentry *root) int err = 0; if (!page) return -ENOMEM; - devname = nfs_path(&dummy, root, page, PAGE_SIZE); + devname = nfs_path(&dummy, root, page, PAGE_SIZE, false); if (IS_ERR(devname)) err = PTR_ERR(devname); else