svn commit: r257638 - head/cddl/contrib/opensolaris/lib/libnvpair

Bruce Evans brde at optusnet.com.au
Tue Nov 5 01:33:39 UTC 2013


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


More information about the svn-src-all mailing list