[PATCH 1/1] ip-link: in human readable output use dynamic precision length

[PATCH 1/1] ip-link: in human readable output use dynamic precision length

[Christian Hesse]

Now that we use floating point numbers for human readable output we can
calculate precision length on the fly.
---
 ip/ipaddress.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index e240bb5..0ddcb0d 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -343,8 +343,8 @@ static void print_num(FILE *fp, unsigned width, uint64_t count)
 		++prefix;
 	}
 
-	snprintf(buf, sizeof(buf), "%.1f%c%s", (double) count / powi, 
-		 *prefix, use_iec ? "i" : "");
+	snprintf(buf, sizeof(buf), "%.*f%c%s", 3 - snprintf(NULL, 0, "%"PRIu64, count / powi),
+		(double) count / powi, *prefix, use_iec ? "i" : "");
 
 	fprintf(fp, "%-*s ", width, buf);
 }
-- 
2.1.3

Re: [PATCH 1/1] ip-link: in human readable output use dynamic precision length

[Christian Hesse]

[-- Attachment #1: Type: text/plain, Size: 969 bytes --]

Christian Hesse <mail@eworm.de> on Tue, 2014/11/04 10:39:
> Now that we use floating point numbers for human readable output we can
> calculate precision length on the fly.
> ---
>  ip/ipaddress.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index e240bb5..0ddcb0d 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -343,8 +343,8 @@ static void print_num(FILE *fp, unsigned width,
> uint64_t count) ++prefix;
>  	}
>  
> -	snprintf(buf, sizeof(buf), "%.1f%c%s", (double) count / powi, 
> -		 *prefix, use_iec ? "i" : "");
> +	snprintf(buf, sizeof(buf), "%.*f%c%s", 3 - snprintf(NULL, 0,
> "%"PRIu64, count / powi),
> +		(double) count / powi, *prefix, use_iec ? "i" : "");
>  
>  	fprintf(fp, "%-*s ", width, buf);
>  }

Damn... In IEC mode we have negative precision length for values between 1000
and 1024. Will send a new patch.
-- 
Best Regards,
Christian Hesse

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v2 1/1] ip-link: in human readable output use dynamic precision length

[Christian Hesse]

---
 ip/ipaddress.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index e240bb5..55cbc77 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -324,6 +324,7 @@ static void print_num(FILE *fp, unsigned width, uint64_t count)
 	const char *prefix = "kMGTPE";
 	const unsigned int base = use_iec ? 1024 : 1000;
 	uint64_t powi = 1;
+	int precision;
 	char buf[64];
 
 	if (!human_readable || count < base) {
@@ -343,8 +344,11 @@ static void print_num(FILE *fp, unsigned width, uint64_t count)
 		++prefix;
 	}
 
-	snprintf(buf, sizeof(buf), "%.1f%c%s", (double) count / powi, 
-		 *prefix, use_iec ? "i" : "");
+	if ((precision = 3 - snprintf(NULL, 0, "%"PRIu64, count / powi)) < 0)
+		precision = 0;
+
+	snprintf(buf, sizeof(buf), "%.*f%c%s", precision,
+		(double) count / powi, *prefix, use_iec ? "i" : "");
 
 	fprintf(fp, "%-*s ", width, buf);
 }
-- 
2.1.3

RE: [PATCH v2 1/1] ip-link: in human readable output use dynamic precision length

[David Laight]

From: Christian Hesse
...
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index e240bb5..55cbc77 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -324,6 +324,7 @@ static void print_num(FILE *fp, unsigned width, uint64_t count)
>  	const char *prefix = "kMGTPE";
>  	const unsigned int base = use_iec ? 1024 : 1000;
>  	uint64_t powi = 1;
> +	int precision;
>  	char buf[64];
> 
>  	if (!human_readable || count < base) {
> @@ -343,8 +344,11 @@ static void print_num(FILE *fp, unsigned width, uint64_t count)
>  		++prefix;
>  	}
> 
> -	snprintf(buf, sizeof(buf), "%.1f%c%s", (double) count / powi,
> -		 *prefix, use_iec ? "i" : "");
> +	if ((precision = 3 - snprintf(NULL, 0, "%"PRIu64, count / powi)) < 0)

Don't put assignments in conditionals.

> +		precision = 0;
> +
> +	snprintf(buf, sizeof(buf), "%.*f%c%s", precision,
> +		(double) count / powi, *prefix, use_iec ? "i" : "");
> 
>  	fprintf(fp, "%-*s ", width, buf);
>  }
> --
> 2.1.3

The above will go wrong in all sorts of horrid ways....
For instance you are doing a truncating integer divide, but the FP
value will get rounded for display.

It would be safer to use integers throughout.

Oh, and a 2Mbit E1 link is actually 2048000 :-)

	David

Re: [PATCH v2 1/1] ip-link: in human readable output use dynamic precision length

[Christian Hesse]

[-- Attachment #1: Type: text/plain, Size: 1484 bytes --]

David Laight <David.Laight@ACULAB.COM> on Tue, 2014/11/04 11:06:
> From: Christian Hesse
> ...
> > ...
> > @@ -343,8 +344,11 @@ static void print_num(FILE *fp, unsigned width,
> > uint64_t count) ++prefix;
> >  	}
> > 
> > -	snprintf(buf, sizeof(buf), "%.1f%c%s", (double) count / powi,
> > -		 *prefix, use_iec ? "i" : "");
> > +	if ((precision = 3 - snprintf(NULL, 0, "%"PRIu64, count / powi))
> > < 0)
> 
> Don't put assignments in conditionals.

Ok. :D

I do not like this at all... snprintf() would be nice for a catch-all, but we
have to take care of negative values. So let's try something different.
I will think about it and send a new patch.

> > +		precision = 0;
> > +
> > +	snprintf(buf, sizeof(buf), "%.*f%c%s", precision,
> > +		(double) count / powi, *prefix, use_iec ? "i" : "");
> > 
> >  	fprintf(fp, "%-*s ", width, buf);
> >  }
> 
> The above will go wrong in all sorts of horrid ways....
> For instance you are doing a truncating integer divide, but the FP
> value will get rounded for display.
> 
> It would be safer to use integers throughout.

My implementation used integers, but Stephen changes this to floating point
with his cleanups.

IMHO the rounding is ok. This is for *human* readability. ;)
Whoever wants correct values should not ask ip to print human readable values
but rely on pure numbers.

> Oh, and a 2Mbit E1 link is actually 2048000 :-)

Sorry? Did not get the point.
-- 
Best regards,
Chris

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v3 1/1] ip-link: in human readable output use dynamic precision length

[Christian Hesse]

---
 ip/ipaddress.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index e240bb5..db39437 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -324,6 +324,8 @@ static void print_num(FILE *fp, unsigned width, uint64_t count)
 	const char *prefix = "kMGTPE";
 	const unsigned int base = use_iec ? 1024 : 1000;
 	uint64_t powi = 1;
+	uint16_t powj = 1;
+	uint8_t precision = 2;
 	char buf[64];
 
 	if (!human_readable || count < base) {
@@ -343,8 +345,15 @@ static void print_num(FILE *fp, unsigned width, uint64_t count)
 		++prefix;
 	}
 
-	snprintf(buf, sizeof(buf), "%.1f%c%s", (double) count / powi, 
-		 *prefix, use_iec ? "i" : "");
+	/* try to guess a good number of digits for precision */
+	for (; precision > 0; precision--) {
+		powj *= 10;
+		if (count / powi < powj)
+			break;
+	}
+
+	snprintf(buf, sizeof(buf), "%.*f%c%s", precision,
+		(double) count / powi, *prefix, use_iec ? "i" : "");
 
 	fprintf(fp, "%-*s ", width, buf);
 }
-- 
2.1.3

RE: [PATCH v2 1/1] ip-link: in human readable output use dynamic precision length

[David Laight]

From: Christian Hesse
...
> 
> I do not like this at all... snprintf() would be nice for a catch-all, but we
> have to take care of negative values. So let's try something different.
> I will think about it and send a new patch.
> 
> > > +		precision = 0;
> > > +
> > > +	snprintf(buf, sizeof(buf), "%.*f%c%s", precision,
> > > +		(double) count / powi, *prefix, use_iec ? "i" : "");
> > >
> > >  	fprintf(fp, "%-*s ", width, buf);
> > >  }
> >
> > The above will go wrong in all sorts of horrid ways....
> > For instance you are doing a truncating integer divide, but the FP
> > value will get rounded for display.
> >
> > It would be safer to use integers throughout.
> 
> My implementation used integers, but Stephen changes this to floating point
> with his cleanups.
> 
> IMHO the rounding is ok. This is for *human* readability. ;)
> Whoever wants correct values should not ask ip to print human readable values
> but rely on pure numbers.

Rounding the value is fine, the difference between 1000 and 1024
is not that great either.
The problem is that the two calculations get rounded differently.
If the FP value ends up being 9.999999 it will be printed as 10.0
but your field width calculation will only have allowed for
a single digit.

> > Oh, and a 2Mbit E1 link is actually 2048000 :-)
> 
> Sorry? Did not get the point.
That was really in reference to a much earlier comment about comms
always using 1000.

	David