Btrfs devel - Feb 2013 - [PATCH] Btrfs-progs: fix parse_limit function to return errors when parsing unit

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.

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
---
 cmds-qgroup.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

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;
-- 
1.7.7.6

--
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 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