[PATCH] tc_util: fix incorrect bare number process in get_rate.

[PATCH] tc_util: fix incorrect bare number process in get_rate.

[Li Wei]

As the comment and manpage indicated that the bare number means
bytes per second, so the division is not needed.
---
 tc/tc_util.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tc/tc_util.c b/tc/tc_util.c
index 926ed08..e8d89c1 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -153,7 +153,7 @@ int get_rate(unsigned *rate, const char *str)
 		return -1;
 
 	if (*p == '\0') {
-		*rate = bps / 8.;	/* assume bytes/sec */
+		*rate = bps;	/* assume bytes/sec */
 		return 0;
 	}
 
-- 
1.7.1

Re: [PATCH] tc_util: fix incorrect bare number process in get_rate.

[Stephen Hemminger]

On Wed, 11 Jul 2012 15:24:50 +0800
Li Wei <lw@cn.fujitsu.com> wrote:

> 
> As the comment and manpage indicated that the bare number means
> bytes per second, so the division is not needed.
> ---
>  tc/tc_util.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tc/tc_util.c b/tc/tc_util.c
> index 926ed08..e8d89c1 100644
> --- a/tc/tc_util.c
> +++ b/tc/tc_util.c
> @@ -153,7 +153,7 @@ int get_rate(unsigned *rate, const char *str)
>  		return -1;
>  
>  	if (*p == '\0') {
> -		*rate = bps / 8.;	/* assume bytes/sec */
> +		*rate = bps;	/* assume bytes/sec */
>  		return 0;
>  	}
>  
Thanks for finding this. The documentation, code and comment do
all need to be the same!

But changing the code as you propose would break existing usage
by scripts. Instead, the man page and comment need to change
to match the reality of the existing application.

Re: [PATCH] tc_util: fix incorrect bare number process in get_rate.

[Li Wei]

于 2012-7-11 22:51, Stephen Hemminger 写道:
> On Wed, 11 Jul 2012 15:24:50 +0800
> Li Wei <lw@cn.fujitsu.com> wrote:
> 
>>
>> As the comment and manpage indicated that the bare number means
>> bytes per second, so the division is not needed.
>> ---
>>  tc/tc_util.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/tc/tc_util.c b/tc/tc_util.c
>> index 926ed08..e8d89c1 100644
>> --- a/tc/tc_util.c
>> +++ b/tc/tc_util.c
>> @@ -153,7 +153,7 @@ int get_rate(unsigned *rate, const char *str)
>>  		return -1;
>>  
>>  	if (*p == '\0') {
>> -		*rate = bps / 8.;	/* assume bytes/sec */
>> +		*rate = bps;	/* assume bytes/sec */
>>  		return 0;
>>  	}
>>  
> Thanks for finding this. The documentation, code and comment do
> all need to be the same!
> 
> But changing the code as you propose would break existing usage
> by scripts. Instead, the man page and comment need to change
> to match the reality of the existing application.
> 
> 
Well, I see, I'll send another patch to take care of this.

Thanks,

Wei

[PATCH] tc: man: change man page and comment to confirm to code's behavior.

[Li Wei]

Since the get_rate() code incorrectly interpreted bare number, the
behavior is not the same as man page and comment described.

We need to change the man page and comment for compatible with the
existing usage by scripts.
---
 man/man8/tc.8 |    7 +++++--
 tc/tc_util.c  |    2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/man/man8/tc.8 b/man/man8/tc.8
index 958ab98..f0e5613 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -259,6 +259,9 @@ All parameters accept a floating point number, possibly followed by a unit.
 .P
 Bandwidths or rates can be specified in:
 .TP
+bps
+Bytes per second
+.TP
 kbps
 Kilobytes per second
 .TP
@@ -271,8 +274,8 @@ Kilobits per second
 mbit
 Megabits per second
 .TP
-bps or a bare number
-Bytes per second
+bit or a bare number
+Bits per second
 .P
 Amounts of data can be specified in:
 .TP
diff --git a/tc/tc_util.c b/tc/tc_util.c
index 926ed08..ccf8fa4 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -153,7 +153,7 @@ int get_rate(unsigned *rate, const char *str)
 		return -1;
 
 	if (*p == '\0') {
-		*rate = bps / 8.;	/* assume bytes/sec */
+		*rate = bps / 8.;	/* assume bits/sec */
 		return 0;
 	}
 
-- 
1.7.1

Re: [PATCH] tc: man: change man page and comment to confirm to code's behavior.

[Stephen Hemminger]

On Thu, 12 Jul 2012 09:56:57 +0800
Li Wei <lw@cn.fujitsu.com> wrote:

> 
> Since the get_rate() code incorrectly interpreted bare number, the
> behavior is not the same as man page and comment described.
> 
> We need to change the man page and comment for compatible with the
> existing usage by scripts.
> ---
>  man/man8/tc.8 |    7 +++++--
>  tc/tc_util.c  |    2 +-
>  2 files changed, 6 insertions(+), 3 deletions(-)

Thanks for fixing. Accepted.