| Message ID | 1361449575-1835-1-git-send-email-wangshilong1991@gmail.com (mailing list archive) |
|---|---|
| State | Under Review, archived |
| Headers | show |
On Thu, Feb 21, 2013 at 08:26:15PM +0800, Wang Shilong wrote: > From: Wang Shilong <wangsl-fnst@cn.fujitsu.com> > > Steps to reproduce: > btrfs qgroup limit m <mnt>/subv > > Here, unit(k/K/g/G/m/M/t/T) all will trigger the problem. > For the above command, the original code will parse the limit value as 0 > and return successfully.It is wrong,fix it. I don't think we should allow to accept bare units for specifing numbers, so in that case fail with 'unrecognized limit'. david -- 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 2/27/13 9:52 AM, David Sterba wrote: > On Thu, Feb 21, 2013 at 08:26:15PM +0800, Wang Shilong wrote: >> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com> >> >> Steps to reproduce: >> btrfs qgroup limit m <mnt>/subv >> >> Here, unit(k/K/g/G/m/M/t/T) all will trigger the problem. >> For the above command, the original code will parse the limit value as 0 >> and return successfully.It is wrong,fix it. > > I don't think we should allow to accept bare units for specifing > numbers, so in that case fail with 'unrecognized limit'. I agree with that. Maybe since David didn't quite want to take my patch 01/17, and Zach thought we needed better code for it all, and Goffredo didn't like the exit() from parse_size, and now this problem, we need a single, concerted effort to fix up size parsing. Who knew it was so hard? :) -Eric > david > -- > 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
Hello, Eric, David 2013/2/27 Eric Sandeen <sandeen@redhat.com>: > On 2/27/13 9:52 AM, David Sterba wrote: >> On Thu, Feb 21, 2013 at 08:26:15PM +0800, Wang Shilong wrote: >>> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com> >>> >>> Steps to reproduce: >>> btrfs qgroup limit m <mnt>/subv >>> >>> Here, unit(k/K/g/G/m/M/t/T) all will trigger the problem. >>> For the above command, the original code will parse the limit value as 0 >>> and return successfully.It is wrong,fix it. >> >> I don't think we should allow to accept bare units for specifing >> numbers, so in that case fail with 'unrecognized limit'. > > I agree with that. > > Maybe since David didn't quite want to take my patch 01/17, and Zach > thought we needed better code for it all, and Goffredo didn't like > the exit() from parse_size, and now this problem, we need a single, > concerted effort to fix up size parsing. I Agre exit() should not be used in parse_size... But i don't agree with Zach about better code for it... Maybe like this: int parse_size(const char *s, u64 * size).. Many places that use the function parse_size/parse_limit, they have to be corrected after change.... Thanks, Wang > > Who knew it was so hard? :) > > -Eric > >> david >> -- >> 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
diff --git a/cmds-qgroup.c b/cmds-qgroup.c index 1525c11..c75ef9b 100644 --- a/cmds-qgroup.c +++ b/cmds-qgroup.c @@ -214,6 +214,8 @@ static int parse_limit(const char *p, unsigned long long *s) { char *endptr; unsigned long long size; + int len = strlen(p); + int unit = 0; if (strcasecmp(p, "none") == 0) { *s = 0; @@ -232,6 +234,7 @@ static int parse_limit(const char *p, unsigned long long *s) size *= 1024; case 'K': case 'k': + unit = 1; size *= 1024; ++endptr; break; @@ -241,7 +244,11 @@ static int parse_limit(const char *p, unsigned long long *s) return 0; } - if (*endptr) + /* + * (len==1 && unit) means + * limit passed is like k,K,m,M,g,G,t,T. + */ + if (*endptr || (len == 1 && unit)) return 0; *s = size;