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

[Christian Hesse <mail@eworm.de>]

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