[Git][NTPsec/ntpsec][master] Revert "Replace vint64 union with bit-banging macros."

[Eric S. Raymond]

Eric S. Raymond pushed to branch master at NTPsec / ntpsec


Commits:
4db1b76e by Eric S. Raymond at 2016-12-11T10:20:06-05:00
Revert "Replace vint64 union with bit-banging macros."

It produced inexplicably correct behavior with reversed assumptions.

- - - - -


2 changed files:

- include/ntp_types.h
- tests/libntp/vi64ops.c


Changes:

=====================================
include/ntp_types.h
=====================================
--- a/include/ntp_types.h
+++ b/include/ntp_types.h
@@ -43,36 +43,44 @@
 
 /*
  * We now assume the platform supports a 64-bit scalar type (the ISC
- * library wouldn't compile otherwise).
+ * library wouldn't compile otherwise). Sadly, getting rid of vint64
+ * is not as simple as turning it into a scalar due to same strange
+ * code in the calendar calculations.
  */
 
-typedef uint64_t vint64;
-#define LAST32MASK	0x00000000ffffffffUL
-#define FIRST32MASK	0xffffffff00000000UL
-#define GET32LAST(n)	((n) & LAST32MASK)
-#define SET32LAST(n, v) (n) = (((n) & FIRST32MASK) | ((v) & LAST32MASK))
-#define GET32FIRST(n)	((n) >> 32)
-#define SET32FIRST(n,v) (n) = ((((v) & LAST32MASK) << 32) | ((n) & LAST32MASK))
-#ifdef WORDS_BIGENDIAN
-#define vint64lo(n)       ((uint32_t)GET32FIRST(n))
-#define setvint64lo(n,v)  SET32FIRST(n,v)
-#define vint64his(n)      ((int32_t)(GET32LAST(n)))
-#define setvint64his(n,v) SET32LAST(n,v)
-#define vint64hiu(n)      ((uint32_t)(GET32LAST(n)))
-#define setvint64hiu(n,v) SET32LAST(n,v)
-#else
-#define vint64lo(n)       ((uint32_t)GET32LAST(n))
-#define setvint64lo(n,v)  SET32LAST(n,v)
-#define vint64his(n)      ((int32_t)(GET32FIRST(n)))
-#define setvint64his(n,v) SET32FIRST(n,v)
-#define vint64hiu(n)      ((uint32_t)(GET32FIRST(n)))
-#define setvint64hiu(n,v) SET32FIRST(n,v)
-#endif
-#define vint64s(n)        ((int64_t)(n))
-#define setvint64s(n,v)   (n) = ((int64_t)(v))
-#define vint64u(n)        (n)
-#define setvint64u(n,v)   (n) = (v)
-#define negvint64(n)      (n = ((uint64_t)((((int64_t)(n)) * -1))))
+typedef union {
+#   ifdef WORDS_BIGENDIAN
+	struct {
+	        int32_t hi; uint32_t lo;
+	} d_s;
+	struct {
+		uint32_t hi; uint32_t lo;
+	} D_s;
+#   else
+	struct {
+		uint32_t lo;   int32_t hi;
+	} d_s;
+	struct {
+		uint32_t lo; uint32_t hi;
+	} D_s;
+#   endif
+
+	int64_t	q_s;	/*   signed quad scalar */
+	uint64_t Q_s;	/* unsigned quad scalar */
+} vint64; /* variant int 64 */
+
+/* hide the structure of a vint64 */
+#define vint64lo(n)       (n).d_s.lo
+#define setvint64lo(n,v)  (n).d_s.lo = (v)
+#define vint64his(n)      (n).d_s.hi
+#define setvint64his(n,v) (n).d_s.hi = (v)
+#define vint64hiu(n)      (n).D_s.hi
+#define setvint64hiu(n,v) (n).D_s.hi = (v)
+#define vint64s(n)        (n).q_s
+#define setvint64s(n,v)   (n).q_s = (v)
+#define vint64u(n)        (n).Q_s
+#define setvint64u(n,v)   (n).Q_s = (v)
+#define negvint64(n)      (n).q_s *= -1
 
 typedef uint16_t	associd_t; /* association ID */
 #define ASSOCID_MAX	USHRT_MAX


=====================================
tests/libntp/vi64ops.c
=====================================
--- a/tests/libntp/vi64ops.c
+++ b/tests/libntp/vi64ops.c
@@ -25,7 +25,7 @@ bool IsEqual(const vint64 *expected, const vint64 *actual) {
 // ----------------------------------------------------------------------
 // test number parser
 TEST(vi64ops, ParseVUI64_pos) {
-	vint64 act, exp = 0;
+	vint64 act, exp;
 	const char *sp;
 	char       *ep;
 
@@ -38,7 +38,7 @@ TEST(vi64ops, ParseVUI64_pos) {
 }
 
 TEST(vi64ops, ParseVUI64_neg) {
-	vint64 act, exp = 0;
+	vint64 act, exp;
 	const char *sp;
 	char       *ep;
 
@@ -51,7 +51,7 @@ TEST(vi64ops, ParseVUI64_neg) {
 }
 
 TEST(vi64ops, ParseVUI64_case) {
-	vint64 act, exp = 0;
+	vint64 act, exp;
 	const char *sp;
 	char       *ep;
 
@@ -64,7 +64,7 @@ TEST(vi64ops, ParseVUI64_case) {
 }
 
 TEST(vi64ops, HiLoVUI64uh) {
-	vint64 exp = 0;
+	vint64 exp;
 
 	setvint64hiu(exp, 0x01234567);
 	setvint64lo(exp, 0x89ABCDEF);
@@ -72,7 +72,7 @@ TEST(vi64ops, HiLoVUI64uh) {
 }
 
 TEST(vi64ops, HiLoVUI64ul) {
-	vint64 exp = 0;
+	vint64 exp;
 
 	setvint64hiu(exp, 0x01234567);
 	setvint64lo(exp, 0x89ABCDEF);
@@ -80,7 +80,7 @@ TEST(vi64ops, HiLoVUI64ul) {
 }
 
 TEST(vi64ops, HiLoVUI64sh) {
-	vint64 exp = 0;
+	vint64 exp;
 
 	setvint64his(exp, 0x01234567);
 	setvint64lo(exp, 0x89ABCDEF);
@@ -88,7 +88,7 @@ TEST(vi64ops, HiLoVUI64sh) {
 }
 
 TEST(vi64ops, HiLoVUI64sl) {
-	vint64 exp = 0;
+	vint64 exp;
 
 	setvint64his(exp, 0x01234567);
 	setvint64lo(exp, 0x89ABCDEF);
@@ -96,28 +96,28 @@ TEST(vi64ops, HiLoVUI64sl) {
 }
 
 TEST(vi64ops, SetVUI64s_pos) {
-	vint64 exp = 0;
+	vint64 exp;
 
 	setvint64s(exp, 0x0123456789ABCDEF);
 	TEST_ASSERT_EQUAL(vint64s(exp), 81985529216486895);
 }
 
 TEST(vi64ops, SetVUI64s_neg) {
-	vint64 exp = 0;
+	vint64 exp;
 
 	setvint64s(exp, 0xFEDCBA9876543210);
 	TEST_ASSERT_EQUAL(vint64s(exp), -81985529216486896);
 }
 
 TEST(vi64ops, SetVUI64u) {
-	vint64 exp = 0;
+	vint64 exp;
 
 	setvint64u(exp, 0xFEDCBA9876543210);	/* sign bit is on */
 	TEST_ASSERT_EQUAL(vint64s(exp), 18364758544493064720UL);
 }
 
 TEST(vi64ops, NegVUI64) {
-	vint64 exp = 0;
+	vint64 exp;
 
 	setvint64s(exp, 71985529216486896);
 	TEST_ASSERT_EQUAL(negvint64(exp), -71985529216486896);



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/4db1b76eec8b969fadb90c2b5dac456975b3e1b0
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ntpsec.org/pipermail/vc/attachments/20161211/859e4c52/attachment.html>