On Thu, 23 Nov 2017, Konstantin Belousov wrote:
On Thu, Nov 23, 2017 at 04:24:13AM +1100, Bruce Evans wrote:
sysctl/sysctl.c:
sysctl(8) has bogus support for prettyprinting struct vmtotal (sysctl
shouldn't have any prettyprinting, especially not for structs that have
specialized programs to print them and much more). This uses intmax_t
for all calculations and printing except for the int16_t fields, so it
automatically benefited from the expansion. However since it uses a
correct type (signed, and not restricted to 64 bits), it now has minor
type errors -- it dowcasts the uint64_t to intmax_t wheen the latter is
64 bits. Its Makefile uses a fairly high WARNS, so if its type errors
were fatal then printf format checking would have detected them (but
not non-fatal errors involving downcasting).
Below is the cast to uintmax_t and unsigned format for sysctl(8).
This adds style bugs by expanding lines from length 79 to 81.
diff --git a/sbin/sysctl/sysctl.c b/sbin/sysctl/sysctl.c
index e1bf4e31914..92685a8171b 100644
--- a/sbin/sysctl/sysctl.c
+++ b/sbin/sysctl/sysctl.c
@@ -625,15 +625,15 @@ S_vmtotal(size_t l2, void *p)
"%hd Sleep: %hd)\n",
v->t_rq, v->t_dw, v->t_pw, v->t_sl);
printf(
- "Virtual Memory:\t\t(Total: %jdK Active: %jdK)\n",
- (intmax_t)v->t_vm * pageKilo, (intmax_t)v->t_avm * pageKilo);
- printf("Real Memory:\t\t(Total: %jdK Active: %jdK)\n",
- (intmax_t)v->t_rm * pageKilo, (intmax_t)v->t_arm * pageKilo);
- printf("Shared Virtual Memory:\t(Total: %jdK Active: %jdK)\n",
- (intmax_t)v->t_vmshr * pageKilo, (intmax_t)v->t_avmshr * pageKilo);
- printf("Shared Real Memory:\t(Total: %jdK Active: %jdK)\n",
- (intmax_t)v->t_rmshr * pageKilo, (intmax_t)v->t_armshr * pageKilo);
- printf("Free Memory:\t%jdK", (intmax_t)v->t_free * pageKilo);
+ "Virtual Memory:\t\t(Total: %juK Active: %juK)\n",
+ (uintmax_t)v->t_vm * pageKilo, (uintmax_t)v->t_avm * pageKilo);
+ printf("Real Memory:\t\t(Total: %juK Active: %juK)\n",
+ (uintmax_t)v->t_rm * pageKilo, (uintmax_t)v->t_arm * pageKilo);
+ printf("Shared Virtual Memory:\t(Total: %juK Active: %juK)\n",
+ (uintmax_t)v->t_vmshr * pageKilo, (uintmax_t)v->t_avmshr *
pageKilo);
+ printf("Shared Real Memory:\t(Total: %juK Active: %juK)\n",
+ (uintmax_t)v->t_rmshr * pageKilo, (uintmax_t)v->t_armshr *
pageKilo);
+ printf("Free Memory:\t%juK", (uintmax_t)v->t_free * pageKilo);
return (0);
}
All of the casts to uintmax_t can be avoided be avoided by changing the type
of pageKilo from int to uintmax_t.
Better, do the conversion in a function-like macro pgtokb() as is done in
all (?) other utilities, but without so many overflow bugs as in most other
utiities:
#define pgtok(p) ((uintmax_t)(p) * pageKilo)
(pageKilo is back to int). This is still sloppy:
- pageKilo = getpagesize() / 1024 assumes that the page size is a multiple
of 1024 and fails very badly when the page size is 512
- the multiplication can still overflow
- when the size in K is too large to fit, the size in M or G will normally
fit and converting directly to would avoid the overflow assuming that
the page size is <= 1M.
Using floating point (even float precision) avoids all overflow and rounding
problems up to almost 128-bit sizes in bytes:
#define pgtok(p) ((p) * pageKilo) /* any integer type * float */
...
float pageKilo;
...
pageKilo = getpagesize() / 1024;
p = INT128_MAX is slightly too large to convert to float. p =
UINT64_MAX fits in float with reduced precision. No one cares about
printing more that FLT_DIG = 6 digits for memory sizes, but use double
instead of float if you want to fit INT128_MAX and print up to DBL_DIG
= 15 digits.
Then for printing, change all %jd and %ju to %.0f. In, systat, remove all
new printing functions and use putfloat.
The problem of printing large values is still there: putfloat might print
4TB bytes as 4398046511k K's or 4398047M K's (where k means 1000, K means
1024 but M means 1000000 and the K's suffix is implicit, but the new
printing function is little better -- it prints 4294962966K K's or
4194304M K's instead.
Bruce
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
