Fix off-by-one bug in format_time_fraction() introduced by hart@ntp.org.

Remove abs(prec) <= 9 requirement from format_time_fraction().
Add more time{spec|val}_tostr() tests.

bk: 4d4050bbhFJeWxtWxx9_y5W7N_xN3A

diff --git a/include/timetoa.h b/include/timetoa.h
index e766d1300b5539f804e67be82a6fcf96b90e7fd0..686928ab441e715ecbadcf19df1ef75d71cda067 100644 (file)
--- a/include/timetoa.h
+++ b/include/timetoa.h
@@ -65,12 +65,10 @@ typedef unsigned long long u_time;
  *
  * secs - integral seconds of time stamp
  * frac - fractional units
- * prec - log10 of units per second (3=miliseconds, 6=microseconds,..)
+ * prec - log10 of units per second (3=milliseconds, 6=microseconds,..)
  *       or in other words: the number decimal digits required.
- *       If prec is < 0, abs(prec) is taken and for the precision
- *       and 'secs' is treated as an unsigned value.
- *
- * abs(prec) must be in [1 .. 9], or only the seconds are formatted.
+ *       If prec is < 0, abs(prec) is taken for the precision and secs
+ *       is treated as an unsigned value.
  *
  * The function will eventually normalise the fraction and adjust the
  * seconds accordingly.

diff --git a/libntp/timetoa.c b/libntp/timetoa.c
index bcacf095d6e4950aff41643677c71593342a8f22..2f0a7daa387b0ed658abe89d6123df1a77f04866 100644 (file)
--- a/libntp/timetoa.c
+++ b/libntp/timetoa.c
@@ -20,9 +20,11 @@
 
 #include "config.h"
 
+#include <math.h>
 #include <stdio.h>
 
 #include "timetoa.h"
+#include "ntp_assert.h"
 #include "lib_strbuf.h"
 
 /*
@@ -49,19 +51,16 @@ format_time_fraction(
        int     prec
        )
 {
-       static const long limit[10] = {
-               1,
-               10, 100, 1000,
-               10000, 100000, 1000000,
-               10000000, 100000000, 1000000000
-       };
-
        char *          cp;
        u_time          ttmp;   /* unsigned storage for seconds */
+       int             i;
+       long            fraclimit;
        int             notneg; /* flag for non-negative value  */
        const char *    fmt;
        ldiv_t          qr;
 
+       DEBUG_REQUIRE(prec != 0);
+
        LIB_GETBUF(cp);
        ttmp = (u_time)secs;
        fmt = "-%" UTIME_FORMAT ".%0*ld";
@@ -69,43 +68,36 @@ format_time_fraction(
        /* check if we need signed or unsigned mode */
        notneg = (prec < 0);
        prec = abs(prec);
-       if (prec <= 0 || prec > COUNTOF(limit)) {
-               if (notneg)
-                       fmt = "%" UTIME_FORMAT;
-               else
-                       fmt = "%" TIME_FORMAT;
-               snprintf(cp, LIB_BUFLENGTH, fmt, secs);
-
-               return cp;
-       }
+       /* fraclimit = (long)pow(10, prec); */
+       for (fraclimit = 10, i = 1; i < prec; i++)
+               fraclimit *= 10;
+       DEBUG_INSIST(fraclimit > 0);
 
        /*
         * Since conversion to string uses lots of divisions anyway,
         * there's no big extra penalty for normalisation. We do it for
         * consistency.
         */
-       if (frac < 0 || frac >= limit[prec]) {
-               qr = ldiv(frac, limit[prec]);
+       if (frac < 0 || frac >= fraclimit) {
+               qr = ldiv(frac, fraclimit);
                if (qr.rem < 0) {
                        qr.quot--;
-                       qr.rem += limit[prec];
+                       qr.rem += fraclimit;
                }
                ttmp += (time_t)qr.quot;
                frac = qr.rem;
        }
 
-       /*
-        * Get the absolute value of the time stamp.
-        */
+       /* Get the absolute value of the time stamp. */
        notneg = notneg || ((time_t)ttmp >= 0);
-       if (!notneg) {
+       if (notneg) {
+               fmt++; /* skip sign char in format string */
+       } else {
                ttmp = ~ttmp;
                if (frac != 0)
-                       frac = limit[prec] - frac;
+                       frac = fraclimit - frac;
                else
                        ttmp += 1;
-       } else {
-               fmt++; /* skip sign char in format string */
        }
 
        /* finally format the data and return the result */

diff --git a/tests/libntp/tspecops.cpp b/tests/libntp/tspecops.cpp
index 9a57b4550fc0aa8641846fcf1af52ea23d6807a2..008349298719fcd73e26d6c12dc9cb08c9488daf 100644 (file)
--- a/tests/libntp/tspecops.cpp
+++ b/tests/libntp/tspecops.cpp
@@ -347,8 +347,10 @@ TEST_F(timespecTest, ToString) {
                { 2, 0,  "2.000000000" },
                {-2, 0, "-2.000000000" },
                { 0, 1,  "0.000000001" },
+               { 0,-1, "-0.000000001" },
                { 1,-1,  "0.999999999" },
-               {-1, 1, "-0.999999999" }
+               {-1, 1, "-0.999999999" },
+               {-1,-1, "-1.000000001" },
        };
        for (int i = 0; i < COUNTOF(data); i++) {
                TSPEC a(data[i].sec, data[i].nsec);

diff --git a/tests/libntp/tvalops.cpp b/tests/libntp/tvalops.cpp
index 4ca17cd6bc49d359d81c86d42f16cdc935a103c2..cb43123afd35efbb5ee56cce54f7b82dcdde7178 100644 (file)
--- a/tests/libntp/tvalops.cpp
+++ b/tests/libntp/tvalops.cpp
@@ -382,8 +382,10 @@ TEST_F(timevalTest, ToString) {
                { 2, 0,  "2.000000" },
                {-2, 0, "-2.000000" },
                { 0, 1,  "0.000001" },
+               { 0,-1, "-0.000001" },
                { 1,-1,  "0.999999" },
-               {-1, 1, "-0.999999" }
+               {-1, 1, "-0.999999" },
+               {-1,-1, "-1.000001" },
        };
        for (int i = 0; i < COUNTOF(data); ++i) {
                timeval_wrap a(data[i].sec, data[i].usec);