Date: Tue, 5 Nov 2013 12:33:27 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Sean Bruno <sbruno@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r257638 - head/cddl/contrib/opensolaris/lib/libnvpair Message-ID: <20131105114408.B2099@besplex.bde.org> In-Reply-To: <201311041615.rA4GFhW9040552@svn.freebsd.org> References: <201311041615.rA4GFhW9040552@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 4 Nov 2013, Sean Bruno wrote: > Log: > Quiesce warning regarding %llf which has no effect. > > Submitted as illumos issue #4284 > > Reviewed by: delphij > > Modified: > head/cddl/contrib/opensolaris/lib/libnvpair/libnvpair.c > > Modified: head/cddl/contrib/opensolaris/lib/libnvpair/libnvpair.c > ============================================================================== > --- head/cddl/contrib/opensolaris/lib/libnvpair/libnvpair.c Mon Nov 4 15:55:04 2013 (r257637) > +++ head/cddl/contrib/opensolaris/lib/libnvpair/libnvpair.c Mon Nov 4 16:15:43 2013 (r257638) > @@ -210,7 +210,7 @@ NVLIST_PRTFUNC(int32, int32_t, int32_t, > NVLIST_PRTFUNC(uint32, uint32_t, uint32_t, "0x%x") > NVLIST_PRTFUNC(int64, int64_t, longlong_t, "%lld") > NVLIST_PRTFUNC(uint64, uint64_t, u_longlong_t, "0x%llx") > -NVLIST_PRTFUNC(double, double, double, "0x%llf") > +NVLIST_PRTFUNC(double, double, double, "0x%f") > NVLIST_PRTFUNC(string, char *, char *, "%s") > NVLIST_PRTFUNC(hrtime, hrtime_t, hrtime_t, "0x%llx") This still has an obfuscating 0x prefix for floating point. This still has the following printf format errors and style bugs: - wrong ptype for vtype uint32_t. The ptype uint32_t is only accidentally compatible with %x. Depending on it being the same bogotifies the careful design of the API -- ptype exists to convert vtype to exactly the type of the format specifier. - use of explicit "0x" prefix for hex numbers. The "#" flag does this better - use of the long long abomination. Assumptions in this use. The longlong_t typedef exists to hide the unportabilities of the abomination. It is semi-opaque and might not be exactly long long. It is for the ptype when the format is "%lld". The latter uses precisely long long, so there is a type mismatch if and only if the longlong_t type is needed as a typedef. Also, if long long has to be typedefed because it doesn't exist, then "%lld" probably doesn't exist either. I think longlong_t is always long long in cddl, so this is only an obfuscation. It is declared in a cddl header in /sys/. In /sys/, the long long abomination is too often used, although I terminated most uses of it before it was standardized, but outside of cddl, longlong_t is only used in a couple of places in xdr/xdr.c. xdr.c declares it as quad_t, so it differs from long long on 64-bit arches. rpc/xdr.h uses quad_t directly. The correct format for printing int64_t is "%jd" and the correct ptype for that is intmax_t. libvnpair doesn't even support printing the long long type abomination -- it only uses it cast to match the format, and never starts with a long long. - similarly for the unsigned long long abomination. - wrong ptype for vtype hrtime_t. The ptype is the same as the vtype. This assumes that hrtime_t is unsigned long long to work with format "%llx". It is actually signed long long. As above, ptype exists to avoid making such assumptions. - the sign mismatches for hrtime_t might be more than style bugs. Hex isn't a very good format for times. Hex formats are always unsigned, and if hrtime_t actually needs to be signed then it can hold negative values and these should not be printed as raw hex. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131105114408.B2099>