[Qemu-devel] [PATCH] slirp: Use monotonic clock if available

[Qemu-devel] [PATCH] slirp: Use monotonic clock if available

[Ed Swierk]

Calling gettimeofday() to compute a time interval can cause problems if
the system clock jumps forwards or backwards; use
clock_gettime(CLOCK_MONOTONIC) instead if it is available.

Also remove some useless macros.

Signed-off-by: Ed Swierk <eswierk@aristanetworks.com>

---
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 4bc8a9d..3214c29 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -258,25 +258,30 @@ void slirp_cleanup(Slirp *slirp)
 /*
  * curtime kept to an accuracy of 1ms
  */
-#ifdef _WIN32
 static void updtime(void)
 {
+#ifdef _WIN32
     struct _timeb tb;
 
     _ftime(&tb);
 
     curtime = tb.time * 1000 + tb.millitm;
-}
 #else
-static void updtime(void)
-{
+#ifdef CLOCK_MONOTONIC
+    struct timespec tv;
+
+    clock_gettime(CLOCK_MONOTONIC, &tv);
+
+    curtime = tv.tv_sec * 1000 + tv.tv_nsec / 1000000;
+#else
     struct timeval tv;
 
     gettimeofday(&tv, NULL);
 
     curtime = tv.tv_sec * 1000 + tv.tv_usec / 1000;
-}
 #endif
+#endif
+}
 
 void slirp_select_fill(int *pnfds,
                        fd_set *readfds, fd_set *writefds, fd_set *xfds)
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 22058cd..020412c 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -108,10 +108,6 @@ typedef unsigned char u_int8_t;
 #include <arpa/inet.h>
 #endif
 
-#ifdef GETTIMEOFDAY_ONE_ARG
-#define gettimeofday(x, y) gettimeofday(x)
-#endif
-
 /* Systems lacking strdup() definition in <string.h>. */
 #if defined(ultrix)
 char *strdup(const char *);
diff --git a/slirp/slirp_config.h b/slirp/slirp_config.h
index dbc8dfd..5a0e6c1 100644
--- a/slirp/slirp_config.h
+++ b/slirp/slirp_config.h
@@ -190,9 +190,6 @@
 #define NO_UNIX_SOCKETS
 #endif
 
-/* Define if gettimeofday only takes one argument */
-#undef GETTIMEOFDAY_ONE_ARG
-
 /* Define if you have revoke() */
 #undef HAVE_REVOKE
 

Re: [Qemu-devel] [PATCH] slirp: Use monotonic clock if available

[malc]

On Wed, 22 Jul 2009, Ed Swierk wrote:

[..snip..]

> -static void updtime(void)
> -{
> +#ifdef CLOCK_MONOTONIC
> +    struct timespec tv;
> +
> +    clock_gettime(CLOCK_MONOTONIC, &tv);
> +
> +    curtime = tv.tv_sec * 1000 + tv.tv_nsec / 1000000;
> +#else
>      struct timeval tv;
>  
>      gettimeofday(&tv, NULL);
>  
>      curtime = tv.tv_sec * 1000 + tv.tv_usec / 1000;
> -}
>  #endif
> +#endif
> +}

This is wrong on many levels.

Suppose CLOCK_MONOTONIC(which is wrong _POSIX_MONOTONIC_CLOCK ought to
be checked) is defined and you compile your binary on, say, Linux 2.6
and then try to run it on 2.4 (without any fancy RHEL patches for
instance), things wouldn't work, what's worse they wouldn't work
silently, giving no indication what's wrong all because nobody checks
clock_gettime result for failures.

The CLOCK_MONOTONIC presence ought to be checked at runtime
(see sysconf(3) and _POSIX_MONOTONIC_CLOCK)

[..snip..]

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH] slirp: Use monotonic clock if available

[Ed Swierk]

On Wed, Jul 22, 2009 at 11:38 PM, malc<av1474@comtv.ru> wrote:
> This is wrong on many levels.
>
> Suppose CLOCK_MONOTONIC(which is wrong _POSIX_MONOTONIC_CLOCK ought to
> be checked) is defined and you compile your binary on, say, Linux 2.6
> and then try to run it on 2.4 (without any fancy RHEL patches for
> instance), things wouldn't work, what's worse they wouldn't work
> silently, giving no indication what's wrong all because nobody checks
> clock_gettime result for failures.
>
> The CLOCK_MONOTONIC presence ought to be checked at runtime
> (see sysconf(3) and _POSIX_MONOTONIC_CLOCK)

OK, sounds like slirp should just call qemu_get_clock(rt_clock), which
is supposed to deal with all these complications.

--Ed

[Qemu-devel] Re: [PATCH] slirp: Use monotonic clock if available

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 791 bytes --]

Ed Swierk wrote:
> On Wed, Jul 22, 2009 at 11:38 PM, malc<av1474@comtv.ru> wrote:
>> This is wrong on many levels.
>>
>> Suppose CLOCK_MONOTONIC(which is wrong _POSIX_MONOTONIC_CLOCK ought to
>> be checked) is defined and you compile your binary on, say, Linux 2.6
>> and then try to run it on 2.4 (without any fancy RHEL patches for
>> instance), things wouldn't work, what's worse they wouldn't work
>> silently, giving no indication what's wrong all because nobody checks
>> clock_gettime result for failures.
>>
>> The CLOCK_MONOTONIC presence ought to be checked at runtime
>> (see sysconf(3) and _POSIX_MONOTONIC_CLOCK)
> 
> OK, sounds like slirp should just call qemu_get_clock(rt_clock), which
> is supposed to deal with all these complications.

Exactly.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]