Re: Replace LibreSSL times() call

[Joerg Jung]

On 14 Oct 2014, at 22:08, Jonas 'Sortie' Termansen sor...@maxsi.org wrote:

 I noticed libressl's apps.c is using times(3), which is among the functions I 
 am
 aggressively deprecating in my personal system. This patch switches it to use
 the clock_gettime and getrusage instead. I pondered using CLOCK_VIRTUAL rather
 than getrusage, but it turned out to be not be implemented and not portable.
 
 Unfortunately, OS X doesn't have clock_gettime, so the portable version will
 have to add back a times call as a fallback, or perhaps use gettimeofday (but
 this doesn't have the proper time-doesn't-go-backwards semantics).

The OS X specific mach_timebase_info() and mach_absolute_time() 
are may be more appropriate as fallback.

 I didn't use the useful, but non-standard timespecsub and TIMEVAL_TO_TIMESPEC
 macros from sys/time.h to make things easier for the portable version.
 Alternatively they could be used and a fallback implementation can be added to
 the libressl time.h wrapper header.

IMHO the macros are the better choice. 

Regards,
Joerg

 --- libressl-2.1.0/apps/apps.c2014-10-11 18:58:11.0 +0200
 +++ libssl/apps/apps.c2014-10-14 21:31:44.827167386 +0200
 @@ -126,7 +126,6 @@
 
 #include sys/types.h
 #include sys/stat.h
 -#include sys/times.h
 
 #include ctype.h
 #include errno.h
 @@ -135,6 +134,7 @@
 #include limits.h
 #include string.h
 #include strings.h
 +#include time.h
 #include unistd.h
 
 #include apps.h
 @@ -2203,25 +2203,40 @@
 #endif
 /* !OPENSSL_NO_TLSEXT  !OPENSSL_NO_NEXTPROTONEG */
 
 +static struct timespec ts_elapsed(struct timespec a, struct timespec b)
 +{
 + a.tv_sec -= b.tv_sec;
 + a.tv_nsec -= b.tv_nsec;
 + if ( a.tv_nsec  0 )
 + {
 + a.tv_nsec += 10L;
 + a.tv_sec -= 1;
 + }
 + return a;
 +}
 +
 double
 app_tminterval(int stop, int usertime)
 {
 - double ret = 0;
 - struct tms rus;
 - clock_t now = times(rus);
 - static clock_t tmstart;
 + static struct timespec start_ts;
 + struct timespec now_ts;
 
 - if (usertime)
 - now = rus.tms_utime;
 + if (usertime) {
 + struct rusage ru;
 + getrusage(RUSAGE_SELF, ru);
 + now_ts.tv_sec = ru.ru_utime.tv_sec;
 + now_ts.tv_nsec = ru.ru_utime.tv_usec * 1000L;
 + } else {
 + clock_gettime(CLOCK_MONOTONIC, now_ts);
 + }
 
 - if (stop == TM_START)
 - tmstart = now;
 - else {
 - long int tck = sysconf(_SC_CLK_TCK);
 - ret = (now - tmstart) / (double) tck;
 + if (stop == TM_START) {
 + start_ts = now_ts;
 + return 0.0;
   }
 
 - return (ret);
 + struct timespec elapsed_ts = ts_elapsed(now_ts, start_ts);
 + return (double) elapsed_ts.tv_sec + (double) elapsed_ts.tv_nsec / 1E9;
 }
 
 int
 

Re: Replace LibreSSL times() call

[Todd C. Miller]

On Tue, 14 Oct 2014 22:08:20 +0200, Jonas 'Sortie' Termansen wrote:

 Unfortunately, OS X doesn't have clock_gettime, so the portable version will
 have to add back a times call as a fallback, or perhaps use gettimeofday (but
 this doesn't have the proper time-doesn't-go-backwards semantics).

You can emulate clock_gettime() on Mac OS X and use mach_absolute_time()
for CLOCK_MONOTONIC.  Portable could use something like:
http://www.sudo.ws/repos/sudo/file/adf7997a0a65/lib/util/clock_gettime.c

 - todd

Re: Replace LibreSSL times() call

[Philip Guenther]

On Tue, Oct 14, 2014 at 1:08 PM, Jonas 'Sortie' Termansen
sor...@maxsi.org wrote:
 I noticed libressl's apps.c is using times(3), which is among the functions I 
 am
 aggressively deprecating in my personal system.

times() is a standard function (not even deprecated or obsolete in
POSIX).  Simply saying _what_ you want to do without saying *why*
leaves us without any reason to do what you describe.

Indeed, you give reasons to *not* do them:

 This patch switches it to use
 the clock_gettime and getrusage instead. I pondered using CLOCK_VIRTUAL rather
 than getrusage, but it turned out to be not be implemented and not portable.

 Unfortunately, OS X doesn't have clock_gettime, so the portable version will
 have to add back a times call as a fallback, or perhaps use gettimeofday (but
 this doesn't have the proper time-doesn't-go-backwards semantics).

Remove something standard and working and replace it with something
non-standard or using more ifdefs?  Umm, why?

Maybe this is a good idea, but you let out all the here's why! part
in your message.


Philip Guenther

Replace LibreSSL times() call

[Jonas 'Sortie' Termansen]

Hi,

I noticed libressl's apps.c is using times(3), which is among the functions I am
aggressively deprecating in my personal system. This patch switches it to use
the clock_gettime and getrusage instead. I pondered using CLOCK_VIRTUAL rather
than getrusage, but it turned out to be not be implemented and not portable.

Unfortunately, OS X doesn't have clock_gettime, so the portable version will
have to add back a times call as a fallback, or perhaps use gettimeofday (but
this doesn't have the proper time-doesn't-go-backwards semantics).

I didn't use the useful, but non-standard timespecsub and TIMEVAL_TO_TIMESPEC
macros from sys/time.h to make things easier for the portable version.
Alternatively they could be used and a fallback implementation can be added to
the libressl time.h wrapper header.

Jonas

--- libressl-2.1.0/apps/apps.c  2014-10-11 18:58:11.0 +0200
+++ libssl/apps/apps.c  2014-10-14 21:31:44.827167386 +0200
@@ -126,7 +126,6 @@
 
 #include sys/types.h
 #include sys/stat.h
-#include sys/times.h
 
 #include ctype.h
 #include errno.h
@@ -135,6 +134,7 @@
 #include limits.h
 #include string.h
 #include strings.h
+#include time.h
 #include unistd.h
 
 #include apps.h
@@ -2203,25 +2203,40 @@
 #endif
 /* !OPENSSL_NO_TLSEXT  !OPENSSL_NO_NEXTPROTONEG */
 
+static struct timespec ts_elapsed(struct timespec a, struct timespec b)
+{
+   a.tv_sec -= b.tv_sec;
+   a.tv_nsec -= b.tv_nsec;
+   if ( a.tv_nsec  0 )
+   {
+   a.tv_nsec += 10L;
+   a.tv_sec -= 1;
+   }
+   return a;
+}
+
 double
 app_tminterval(int stop, int usertime)
 {
-   double ret = 0;
-   struct tms rus;
-   clock_t now = times(rus);
-   static clock_t tmstart;
+   static struct timespec start_ts;
+   struct timespec now_ts;
 
-   if (usertime)
-   now = rus.tms_utime;
+   if (usertime) {
+   struct rusage ru;
+   getrusage(RUSAGE_SELF, ru);
+   now_ts.tv_sec = ru.ru_utime.tv_sec;
+   now_ts.tv_nsec = ru.ru_utime.tv_usec * 1000L;
+   } else {
+   clock_gettime(CLOCK_MONOTONIC, now_ts);
+   }
 
-   if (stop == TM_START)
-   tmstart = now;
-   else {
-   long int tck = sysconf(_SC_CLK_TCK);
-   ret = (now - tmstart) / (double) tck;
+   if (stop == TM_START) {
+   start_ts = now_ts;
+   return 0.0;
}
 
-   return (ret);
+   struct timespec elapsed_ts = ts_elapsed(now_ts, start_ts);
+   return (double) elapsed_ts.tv_sec + (double) elapsed_ts.tv_nsec / 1E9;
 }
 
 int