lfpinit() signed or unsigned?

[Achim Gratz]

Gary E. Miller writes:
> As I said yesterday, I gave up on the top dopwn approach.  I have
> gone to bottom up.  Fixing small things.  It will prolly take me a week
> to deal with all the really obvious -Wsign-conversion warnings.
>
> I am trying to make all my commits very bytesize, so each is
> trivial to vet.  Feel free to keep an eye on me.  :-)

What became of the idea of you doing that in a branch?  It would really
help to fix the constant stream of typos and "whoops" before they hit
the master branch.  Also, proper review might avoid "improvements"
that really aren't, like this:

--8<---------------cut here---------------start------------->8---
@@ -13,11 +13,22 @@
  * NTP uses two fixed point formats.  The first (l_fp) is the "long"
  * format and is 64 bits wide with the decimal between bits 31 and 32.
  * This is used for time stamps in the NTP packet header (in network
- * byte order) and for internal computations of offsets (in local host
- * byte order). We use the same structure for both signed and unsigned
- * values, which is a big hack but saves rewriting all the operators
- * twice. Just to confuse this, we also sometimes just carry the
- * fractional part in calculations, in both signed and unsigned forms.
+ * byte order).  It is defined in RFC 5905 in Section 6 (Data Types).
+ *
+ * The integral part is unsigned seconds since 0 h 1 January 1900 UTC.
+ * It will overflow in 2036.
+ *
+ * The fractional part is jusr float seconds divided by 32.  Each LSB
+ * is 232 pico seconds.
+ *
+ * For internal computations of offsets (in local host byte order)
+ * the same structure is used, but signed, to allow for positive and
+ * negative offsets. We use the same structure for both signed and
+ * unsigned values, which is a big hack but saves rewriting all the
+ * operators twice. Just to confuse this, we also sometimes just carry
+ * the fractional part in calculations, in both signed and unsigned
+ * forms.
+ *
  * Anyway, an l_fp looks like:
  *
  *    0			  1		      2			  3
--8<---------------cut here---------------end--------------->8---

Again, l_fp is no structure anymore.  It doesn't really consist of an
integral and fractional part either now that it is no longer a
structure, it simply is 64bits in a unit of seconds scaled by 2e-32.
That it was a structure consisting of two 32bit parts is an
implementation detail of how the older code represented NTP time.

There's no "float seconds" and especially none that are "divided by 32"
(which would be a right shift by 5).  The LSB unit would be 233ps if you
round to picosecond precision.

The offset computations don't really use the same structure or even the
same type, but they use the same time unit and are indeed signed.  The
hack the old code used was to not define a separate structure for that
and just stuff the offests and timestamps into the same structure with
oblivion.  There is absolutely no way to treat the lower 32bit
(fractional seconds) directly as a signed type unless you first
determine the sign or direction of the offset and branch based on that
information.


Here's another one:
--8<---------------cut here---------------start------------->8---
@@ -114,6 +114,22 @@ normalize_tspec(
 	return x;
 }
 
+/* convert a double to a rounded and normalized timespec */
+static inline struct timespec
+d_to_tspec(
+	double d
+	)
+{
+	struct timespec	x;
+
+	d += 0.5e-9;	/* round on LSB */
+	x.tv_sec = (time_t) d;
+	x.tv_nsec = (long)((d - (double)x.tv_sec) * 1e9);
+
+        /* unlikely, but ensure it is normalized */
+	return normalize_tspec(x);
+}
+
 /* x = a + b */
 static inline struct timespec
 add_tspec(
--8<---------------cut here---------------end--------------->8---

Sprinkling magical constants throughout the code like that isn't going
to make it easier to maintain (especially when you consider that there's
a myriad ways of writing the same number).  There's a reason NANOSECONDS
gets defined at the top of that file, even though it's unlikely that the
unit of time will change in timespec.  So (0.5/NANOSECONDS) and (… *
NANOSECONDS) would be much easier to find in the event that later on
someone wanted to bump the filed sizes up and scale to attoseconds.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Q+, Q and microQ:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds