| Message ID | 1400519071-5580-1-git-send-email-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 5/19/14, 12:04 PM, Anand Jain wrote: > From: Anand Jain <Anand.Jain@oracle.com> > > generally if you use > echo "test" > /sys/fs/btrfs/<fsid>/label > it would introduce return char at the end and it can not > be part of the label. The correct command is > echo -n "test" > /sys/fs/btrfs/<fsid>/label > > This patch will check for this user error Wouldn't it be a lot better to just strip the "\n" if it exists? > Signed-off-by: Anand Jain <Anand.Jain@oracle.com> > --- > fs/btrfs/sysfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c > index c5eb214..63c2907 100644 > --- a/fs/btrfs/sysfs.c > +++ b/fs/btrfs/sysfs.c > @@ -374,7 +374,7 @@ static ssize_t btrfs_label_store(struct kobject *kobj, > struct btrfs_root *root = fs_info->fs_root; > int ret; > > - if (len >= BTRFS_LABEL_SIZE) { > + if (len >= BTRFS_LABEL_SIZE || strchr(buf, '\n')) { > pr_err("BTRFS: unable to set label with more than %d bytes\n", > BTRFS_LABEL_SIZE - 1); so if I do: # echo "mylabel" > /sys/fs/btrfs/<fsid>/label I'll get: BTRFS: unable to set label with more than 255 bytes" which would be pretty confusing, IMHO, given the short label I tried to create. Just strip out the \n ... -Eric > return -EINVAL; > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 20 May 2014 01:04:30 +0800 Anand Jain <anand.jain@oracle.com> wrote: > From: Anand Jain <Anand.Jain@oracle.com> > > generally if you use > echo "test" > /sys/fs/btrfs/<fsid>/label > it would introduce return char at the end and it can not > be part of the label. The correct command is > echo -n "test" > /sys/fs/btrfs/<fsid>/label > > This patch will check for this user error Maybe instead consider checking for one trailing "\n", and silently remove it if passed, so that both of the mentioned variants of 'echo' can be used? All other sysfs files do not care if you pass an extra "\n" at the end, e.g. echo cfq > /sys/block/sda/queue/scheduler works fine, doesn't require you to use "echo -n cfq".
On 20/05/14 01:16, Eric Sandeen wrote: > On 5/19/14, 12:04 PM, Anand Jain wrote: >> From: Anand Jain <Anand.Jain@oracle.com> >> >> generally if you use >> echo "test" > /sys/fs/btrfs/<fsid>/label >> it would introduce return char at the end and it can not >> be part of the label. The correct command is >> echo -n "test" > /sys/fs/btrfs/<fsid>/label >> >> This patch will check for this user error > > Wouldn't it be a lot better to just strip the "\n" if it > exists? yes. Thanks Eric. >> Signed-off-by: Anand Jain <Anand.Jain@oracle.com> >> --- >> fs/btrfs/sysfs.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c >> index c5eb214..63c2907 100644 >> --- a/fs/btrfs/sysfs.c >> +++ b/fs/btrfs/sysfs.c >> @@ -374,7 +374,7 @@ static ssize_t btrfs_label_store(struct kobject *kobj, >> struct btrfs_root *root = fs_info->fs_root; >> int ret; >> >> - if (len >= BTRFS_LABEL_SIZE) { >> + if (len >= BTRFS_LABEL_SIZE || strchr(buf, '\n')) { >> pr_err("BTRFS: unable to set label with more than %d bytes\n", >> BTRFS_LABEL_SIZE - 1); > > so if I do: > > # echo "mylabel" > /sys/fs/btrfs/<fsid>/label > > I'll get: > > BTRFS: unable to set label with more than 255 bytes" > > which would be pretty confusing, IMHO, given the short > label I tried to create. > > Just strip out the \n ... > > -Eric > >> return -EINVAL; >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20/05/14 01:19, Roman Mamedov wrote: > On Tue, 20 May 2014 01:04:30 +0800 > Anand Jain <anand.jain@oracle.com> wrote: > >> From: Anand Jain <Anand.Jain@oracle.com> >> >> generally if you use >> echo "test" > /sys/fs/btrfs/<fsid>/label >> it would introduce return char at the end and it can not >> be part of the label. The correct command is >> echo -n "test" > /sys/fs/btrfs/<fsid>/label >> >> This patch will check for this user error > > Maybe instead consider checking for one trailing "\n", and silently remove it > if passed, so that both of the mentioned variants of 'echo' can be used? > > All other sysfs files do not care if you pass an extra "\n" at the end, e.g. > > echo cfq > /sys/block/sda/queue/scheduler > > works fine, doesn't require you to use "echo -n cfq". > Thanks Roman. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index c5eb214..63c2907 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -374,7 +374,7 @@ static ssize_t btrfs_label_store(struct kobject *kobj, struct btrfs_root *root = fs_info->fs_root; int ret; - if (len >= BTRFS_LABEL_SIZE) { + if (len >= BTRFS_LABEL_SIZE || strchr(buf, '\n')) { pr_err("BTRFS: unable to set label with more than %d bytes\n", BTRFS_LABEL_SIZE - 1); return -EINVAL;