From: Juergen Perlinger Date: Wed, 19 Jan 2011 19:43:27 +0000 (+0100) Subject: changed timeval / timespec string conversion (snprintf() format strings for time_t... X-Git-Tag: NTP_4_2_7P120~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b6a8150df39899baa320290ff3a6961bbdcf79df;p=thirdparty%2Fntp.git changed timeval / timespec string conversion (snprintf() format strings for time_t vs. int / long / long long; avoid signed integer overflow) bk: 4d373edfXhkbBFI33aUAZayXY3Q8AA --- diff --git a/ChangeLog b/ChangeLog index dc99f3171..649255902 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,6 @@ +* changed timeval / timespec string conversion (snprintf() format + strings for time_t vs. int / long / long long; avoid signed integer + overflow) * Grow ntpd/work_thread.c arrays as needed. * Add DEBUG_* variants of ntp_assert.h macros which compile away using ./configure --disable-debugging. diff --git a/libntp/timespecops.c b/libntp/timespecops.c index b620a901f..849fd3964 100644 --- a/libntp/timespecops.c +++ b/libntp/timespecops.c @@ -24,6 +24,7 @@ #undef NANOSECONDS #define NANOSECONDS 1000000000 +/* conversion between l_fp fractions and nanoseconds */ #if SIZEOF_LONG >= 8 # define MYFTOTVN(tsf,tvu) \ ((tvu) = (int32) \ @@ -39,6 +40,22 @@ ((tsf) = (u_int32)floor((tvu) * 4.294967296 + 0.5)) #endif +/* using snprintf is troublesome with time_t. Try to resolve it. */ +#if SIZEOF_TIME_T <= SIZEOF_INT +typedef unsigned int u_time; +#define TIMEFMT "" +#elif SIZEOF_TIME_T <= SIZEOF_LONG +typedef unsigned long u_time; +#define TIMEFMT "l" +#elif SIZEOF_TIME_T <= SIZEOF_LONG_LONG +typedef unsigned long long u_time; +#define TIMEFMT "ll" +#else +# include "GRONK: what size has a time_t here?" +#endif + + +/* copy and normalise. Used often enough to warrant a macro. */ #define COPYNORM(dst, src) \ do { \ *(dst) = *(src); \ @@ -53,17 +70,17 @@ timespec_norm( ) { #if SIZEOF_LONG > 4 - /* tv_nsec is of type 'long', and on a 64 bit machine - * normalisation by loop becomes prohibitive, once the upper 32 - * bits become involved. On the other hand, division by constant - * should be fast enough; so we do a division of the nanoseconds - * if the high dword becomes involved. The floor adjustment step - * follows with the standard normalisation loops. Also note - * that 'abs' returns int, which might not be good here. */ - long z; - - z = x->tv_nsec; - if ((labs(z) >> 32) > 0) { + /* tv_nsec is of type 'long', and on a 64 bit machine using only + * loops becomes prohibitive once the upper 32 bits get + * involved. On the other hand, division by constant should be + * fast enough; so we do a division of the nanoseconds in that + * case. The floor adjustment step follows with the standard + * normalisation loops. And 'labs' is intentinally not used + * here: its has implementation defined behaviour when applied + * to LONG_MIN. */ + if (x->tv_nsec < -3l * NANOSECONDS || + x->tv_nsec > 3l * NANOSECONDS ) { + long z; z = x->tv_nsec / NANOSECONDS; x->tv_nsec -= z * NANOSECONDS; x->tv_sec += z; @@ -184,10 +201,15 @@ timespec_abs( COPYNORM(&c, a); r = (c.tv_sec < 0); - if (r != 0) - timespec_neg(x, &c); - else - *x = c; + if (r != 0) { + c.tv_sec = - c.tv_sec; + c.tv_nsec = - c.tv_nsec; + if (c.tv_nsec < 0) { + c.tv_sec -= 1; + c.tv_nsec += NANOSECONDS; + } + } + *x = c; return r; } @@ -222,13 +244,18 @@ timespec_cmp( const struct timespec *b ) { + int r; struct timespec A; struct timespec B; COPYNORM(&A, a); COPYNORM(&B, b); + r = (A.tv_sec > B.tv_sec) - (A.tv_sec < B.tv_sec); + if (r == 0) + r = (A.tv_nsec > B.tv_nsec) - + (A.tv_nsec < B.tv_nsec); - return timespec_cmp_fast(&A, &B); + return r; } /* @@ -258,11 +285,15 @@ timespec_test( const struct timespec *a ) { + int r; struct timespec A; COPYNORM(&A, a); + r = (A.tv_sec > 0) - (A.tv_sec < 0); + if (r == 0) + r = (A.tv_nsec > 0); - return timespec_test_fast(&A); + return r; } const char* @@ -272,77 +303,41 @@ timespec_tostr( { /* * Formatting a 'time_t' to a string of decimal digits poses - * some interesting portability problems: - * - * 1.) The size of 'time_t' is not defined: 32, 64, or arbitray - * bits (though 32 and 64 are most common, of course) - * - * 2.) It's not easy to find an unsigned type with the same width of - * time_t in a portable way. (int / long / long long / __int64 - * /...) - * - * 3.) Possible 'printf()' format specs are also difficult to find. - * - * Therefore, this implementation chooses to do the conversion - * manually, doing divisions on a possible negative time_t value - * even after a normalisation step. (-TIME_T_MIN == TIME_T_MIN - * on many systems, though there is no actual definition for - * TIME_T_MIN, but I guess you get my meaning...) - * - * And since we have to do the integer part manually, we do the - * fractional part in the same way, though after the - * normalisation we can be sure that at least the fraction is in - * range. + * some interesting portability problems regarding the size of a + * time_t and the handling of signed integer overflow, which is + * undefined and must be avoided. * * Even with 64 bit time_t, 32 chars will suffice. Hopefully, - * LIB_BUFLENGTH is big enough; the current definiton does this - * check by the preprocessor just at the top of this file. - */ + * LIB_BUFLENGTH is big enough; the current definiton checks + * this by the preprocessor just at the top of this file. */ + static const char *fmt = "-%" TIMEFMT "u.%09lu"; + struct timespec v; - int s; - int digits; - int dig; - char * cp; - time_t itmp; - long ftmp; - - /* normalise to positive value and get sign flag */ - s = timespec_abs(&v, x); - - /* get buffer, position to end and insert terminating NUL */ - LIB_GETBUF(cp); - cp += LIB_BUFLENGTH - 1; - *cp = '\0'; - - /* convert fraction to decimal digits */ - for (digits = 9; digits; digits--) { - ftmp = v.tv_nsec / 10; - dig = (int)(v.tv_nsec - ftmp * 10); - v.tv_nsec = ftmp; - *--cp = '0' + (char)dig; - } - *--cp = '.'; + char *cp; + int notneg; + u_time itmp; + u_long ftmp; - /* convert integer part to decimal digits - * - * -*- convert at least one digit, make sure remaining value - * is not negative. This assumes that integer division - * truncates towards zero, as required by the standard. */ - itmp = v.tv_sec / 10; - dig = (int)(v.tv_sec - itmp * 10); - v.tv_sec = max(-itmp, itmp); - *--cp = '0' + (char)abs(dig); - /* -*- convert remaining digits */ - while (v.tv_sec != 0) { - itmp = v.tv_sec / 10; - dig = (int)(v.tv_sec - itmp * 10); - v.tv_sec = itmp; - *--cp = '0' + (char)dig; + /* normalise and get absolute value into unsigned values. Since + * the negation of TIME_T_MIN (if it existed) is implementation + * defined, we try to avoid it. */ + COPYNORM(&v, x); + notneg = v.tv_sec >= 0; + if (notneg != 0) { + itmp = (u_time)v.tv_sec; + ftmp = (u_long)v.tv_nsec; + } else if (v.tv_nsec != 0) { + itmp = (u_time)-(v.tv_sec + 1); + ftmp = (u_long)(NANOSECONDS - v.tv_nsec); + } else { + itmp = ((u_time) -(v.tv_sec + 1)) + 1; + ftmp = 0; } - /* add minus sign for negative integer part */ - if (s) - *--cp = '-'; + /* get buffer and format data */ + LIB_GETBUF(cp); + snprintf(cp, LIB_BUFLENGTH, fmt + notneg, itmp, ftmp); + return cp; } @@ -372,7 +367,6 @@ timespec_reltolfp( y->l_i = (int32)v.tv_sec; } - void timespec_relfromlfp( struct timespec *y, @@ -385,11 +379,11 @@ timespec_relfromlfp( tmp = *x; neg = L_ISNEG(&tmp); - if (neg) + if (neg != 0) L_NEG(&tmp); MYFTOTVN(x->l_uf, out.tv_nsec); out.tv_sec = x->l_ui; - if (neg) { + if (neg != 0) { out.tv_sec = -out.tv_sec; out.tv_nsec = -out.tv_nsec; } diff --git a/libntp/timevalops.c b/libntp/timevalops.c index 4f76fe555..4699a9c16 100644 --- a/libntp/timevalops.c +++ b/libntp/timevalops.c @@ -24,19 +24,35 @@ #undef MICROSECONDS #define MICROSECONDS 1000000 +/* conversion between l_fp fractions and microseconds */ #if SIZEOF_LONG >= 8 # define MYFTOTVU(tsf, tvu) \ - (tvu) = (int32) \ - (((u_long)(tsf) * MICROSECONDS + 0x80000000) >> 32) + ((tvu) = (int32) \ + (((u_long)(tsf) * MICROSECONDS + 0x80000000) >> 32)) # define MYTVUTOF(tvu, tsf) \ - (tsf) = (u_int32) \ + ((tsf) = (u_int32) \ ((((u_long)(tvu) << 32) + MICROSECONDS / 2) / \ - MICROSECONDS) + MICROSECONDS)) #else # define MYFTOTVU(tsf, tvu) TSFTOTVU(tsf, tvu) # define MYTVUTOF(tvu, tsf) TVUTOTSF(tvu, tsf) #endif +/* using snprintf is troublesome with time_t. Try to resolve it. */ +#if SIZEOF_TIME_T <= SIZEOF_INT +typedef unsigned int u_time; +#define TIMEFMT "" +#elif SIZEOF_TIME_T <= SIZEOF_LONG +typedef unsigned long u_time; +#define TIMEFMT "l" +#elif SIZEOF_TIME_T <= SIZEOF_LONG_LONG +typedef unsigned long long u_time; +#define TIMEFMT "ll" +#else +# include "GRONK: what size has a time_t here?" +#endif + +/* copy and normalise. Used often enough to warrant a macro. */ #define COPYNORM(dst, src) \ do { \ *(dst) = *(src); \ @@ -52,11 +68,13 @@ timeval_norm( { /* If the fraction becomes excessive denormal, we use division * to do first partial normalisation. The normalisation loops - * following will do the remaining cleanup. Also note that - * 'abs' returns int, which might not be good here. */ - long z; - z = x->tv_usec; - if (max(-z, z) > 3 * MICROSECONDS) { + * following will do the remaining cleanup. Since the size of + * tv_usec has a peculiar definition by the standard the range + * check is coded manualla. + */ + if (x->tv_usec < -3l * MICROSECONDS || + x->tv_usec > 3l * MICROSECONDS ) { + long z; z = x->tv_usec / MICROSECONDS; x->tv_usec -= z * MICROSECONDS; x->tv_sec += z; @@ -176,10 +194,15 @@ timeval_abs( COPYNORM(&c, a); r = (c.tv_sec < 0); - if (r) - timeval_neg(x, &c); - else - *x = c; + if (r != 0) { + c.tv_sec = - c.tv_sec; + c.tv_usec = - c.tv_usec; + if (c.tv_usec < 0) { + c.tv_sec -= 1; + c.tv_usec += MICROSECONDS; + } + } + *x = c; return r; } @@ -209,12 +232,18 @@ timeval_cmp( const struct timeval *b ) { + int r; struct timeval A; struct timeval B; COPYNORM(&A, a); COPYNORM(&B, b); - return timeval_cmp_fast(&A, &B); + r = (A.tv_sec > B.tv_sec) - (A.tv_sec < B.tv_sec); + if (r == 0) + r = (A.tv_usec > B.tv_usec) - + (A.tv_usec < B.tv_usec); + + return r; } /* @@ -245,57 +274,55 @@ timeval_test( ) { struct timeval A; + int r; COPYNORM(&A, a); - return timeval_test_fast(&A); + r = (A.tv_sec > 0) - (A.tv_sec < 0); + if (r == 0) + r = (A.tv_usec > 0); + + return r; } /* return LIB buffer ptr to string rep */ -const char * +const char* timeval_tostr( const struct timeval *x ) { - /* see timespecops.c for rationale -- this needs refactoring */ - struct timeval v; - int s; - int digits; - int dig; + /* see timespecops.c for rationale -- this needs refactoring + * + * Even with 64 bit time_t, 32 chars will suffice. Hopefully, + * LIB_BUFLENGTH is big enough; the current definiton checks + * this by the preprocessor just at the top of this file. */ + static const char *fmt = "-%" TIMEFMT "u.%06lu"; + + struct timeval v; char * cp; - time_t itmp; - long ftmp; - - s = timeval_abs(&v, x); - - LIB_GETBUF(cp); - cp += LIB_BUFLENGTH - 1; - *cp = '\0'; - - /* convert fraction to decimal digits */ - for (digits = 6; digits; digits--) { - ftmp = v.tv_usec / 10; - dig = (int)(v.tv_usec - ftmp * 10); - v.tv_usec = ftmp; - *--cp = '0' + (char)dig; - } - *--cp = '.'; - - /* convert first digit */ - itmp = v.tv_sec / 10; - dig = (int)(v.tv_sec - itmp * 10); - v.tv_sec = max(-itmp, itmp); - *--cp = '0' + (char)abs(dig); - /* -*- convert remaining digits */ - while (v.tv_sec != 0) { - itmp = v.tv_sec / 10; - dig = (int)(v.tv_sec - itmp * 10); - v.tv_sec = itmp; - *--cp = '0' + (char)dig; + int notneg; + u_time itmp; + u_long ftmp; + + /* normalise and get absolute value into unsigned values. Since + * the negation of TIME_T_MIN (if it existed) is implementation + * defined, we try to avoid it. */ + COPYNORM(&v, x); + notneg = v.tv_sec >= 0; + if (notneg != 0) { + itmp = (u_time)v.tv_sec; + ftmp = (u_long)v.tv_usec; + } else if (v.tv_usec != 0) { + itmp = (u_time)-(v.tv_sec + 1); + ftmp = (u_long)(MICROSECONDS - v.tv_usec); + } else { + itmp = ((u_time) -(v.tv_sec + 1)) + 1; + ftmp = 0; } - /* add minus sign for negative integer part */ - if (s) - *--cp = '-'; + /* get buffer and format data */ + LIB_GETBUF(cp); + snprintf(cp, LIB_BUFLENGTH, fmt + notneg, itmp, ftmp); + return cp; } @@ -325,7 +352,6 @@ timeval_reltolfp( y->l_i = (int32)v.tv_sec; } - void timeval_relfromlfp( struct timeval *y, @@ -338,11 +364,11 @@ timeval_relfromlfp( tmp = *x; neg = L_ISNEG(&tmp); - if (neg) + if (neg != 0) L_NEG(&tmp); MYFTOTVU(x->l_uf, out.tv_usec); out.tv_sec = x->l_ui; - if (neg) { + if (neg != 0) { out.tv_sec = -out.tv_sec; out.tv_usec = -out.tv_usec; }