setitimer API is obsolescent

setitimer API is obsolescent

[Paul Eggert]

POSIX made the getitimer/setitimer API obsolescent a decade ago, and now's a 
good time for Emacs to rely on the current timer_gettime/timer_settime API 
instead. I've proposed patches for this in Bug#32746. If you know a platform 
that is a current porting target of Emacs and lacks timer_gettime/timer_settime 
but has getitimer/setitimer, please send email to <32746@debbugs.gnu.org> about 
it. (MS-Windows lacks both APIs; the proposed patches attempt to address that 
special case.)

bug#32746: setitimer API is obsolescent

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 16 Sep 2018 13:32:33 -0700
> 
> POSIX made the getitimer/setitimer API obsolescent a decade ago, and now's a 
> good time for Emacs to rely on the current timer_gettime/timer_settime API 
> instead. I've proposed patches for this in Bug#32746. If you know a platform 
> that is a current porting target of Emacs and lacks timer_gettime/timer_settime 
> but has getitimer/setitimer, please send email to <32746@debbugs.gnu.org> about 
> it. (MS-Windows lacks both APIs; the proposed patches attempt to address that 
> special case.)

Thanks, I will send my comments to the MS-Windows part of the patch in
a moment.

However, I think it might be too early to remove support for
setitimer, because some platforms have problems with it.  AFAIK, macOS
and some *BSD version either don't have that family or have buggy
implementations.

Supporting the old APIs only needs a couple of #ifdef's in 3 files, so
it doesn't sound like a terrible maintenance burden.

Btw, it would be good to have some tests for these facilities, as
currently we don't have any practical means to make sure we don't
introduce bugs when we switch to alternative APIs.

Re: setitimer API is obsolescent

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 16 Sep 2018 13:32:33 -0700
> 
> POSIX made the getitimer/setitimer API obsolescent a decade ago, and now's a 
> good time for Emacs to rely on the current timer_gettime/timer_settime API 
> instead. I've proposed patches for this in Bug#32746. If you know a platform 
> that is a current porting target of Emacs and lacks timer_gettime/timer_settime 
> but has getitimer/setitimer, please send email to <32746@debbugs.gnu.org> about 
> it. (MS-Windows lacks both APIs; the proposed patches attempt to address that 
> special case.)

Thanks, I will send my comments to the MS-Windows part of the patch in
a moment.

However, I think it might be too early to remove support for
setitimer, because some platforms have problems with it.  AFAIK, macOS
and some *BSD version either don't have that family or have buggy
implementations.

Supporting the old APIs only needs a couple of #ifdef's in 3 files, so
it doesn't sound like a terrible maintenance burden.

Btw, it would be good to have some tests for these facilities, as
currently we don't have any practical means to make sure we don't
introduce bugs when we switch to alternative APIs.

bug#32746: setitimer API is obsolescent

[Paul Eggert]

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

Eli Zaretskii wrote:

> However, I think it might be too early to remove support for
> setitimer, because some platforms have problems with it.  AFAIK, macOS
> and some *BSD version either don't have that family or have buggy
> implementations.

In that case never mind about removing use of setitimer. I'm attaching a patch 
against my previous proposal for fixup on the MS-Windows side, in the light of 
your other comments; good luck with it, and feel free to ignore the whole thing 
if it doesn't work. As before, it's untested and no doubt it has problems.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Patch-inspired-by-Eli-s-review-Bug-32764-10.patch --]
[-- Type: text/x-patch; name="0001-Patch-inspired-by-Eli-s-review-Bug-32764-10.patch", Size: 4029 bytes --]

From b52b0505bb53fa00943847eabc60b096bbbdfb80 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 17 Sep 2018 09:04:06 -0700
Subject: [PATCH] =?UTF-8?q?Patch=20inspired=20by=20Eli=E2=80=99s=20review?=
 =?UTF-8?q?=20(Bug#32764#10)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* nt/inc/sys/time.h (pthread_attr_t): Remove.
(struct sigevent): Remove sigev_notify_attributes.
(CLOCK_REALTIME, clockid_t):
Define only if CLOCK_REALTIME isn’t already defined.
(clockid_t): Just use int rather than getting fancy.
(CLOCK_REALTIME, CLOCK_THREAD_CPUTIME_ID):
Just use a macro rather than getting fancy.
(CLOCK_THREAD_CPUTIME_ID): Define to an unlikely int
instead of getting fancy.
* nt/mingw-cfg.site (ac_cv_func_timer_settime): Set to yes.
* src/w32proc.c (timer_gettime): Remove unused local and stray
use of it.
(timer_settime): Fix use of undeclared var.
Reverse sense of TIMER_ABSTIME.
---
 nt/inc/sys/time.h | 15 ++++++++++-----
 nt/mingw-cfg.site |  2 ++
 src/w32proc.c     | 10 ++++------
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/nt/inc/sys/time.h b/nt/inc/sys/time.h
index d03bc456ea..b238e36967 100644
--- a/nt/inc/sys/time.h
+++ b/nt/inc/sys/time.h
@@ -9,7 +9,6 @@ struct itimerspec
   struct  timespec it_value;	/* current value */
 };
 
-typedef struct pthread_attr_t *pthread_attr_t;
 union sigval
 {
   int sival_int;
@@ -21,14 +20,20 @@ struct sigevent
   int sigev_signo;
   union sigval sigev_value;
   void (*sigev_notify_function) (union sigval);
-  pthread_attr_t *sigev_notify_attributes;
+  /* Although POSIX requires 'pthread_attr_t *sigev_notify_attributes;',
+     that might clash with MinGW and Emacs *doesn’t need it.  */
 };
 #define SIGEV_SIGNAL 0
 
 #define TIMER_ABSTIME 1
-typedef enum { CLOCK_REALTIME, CLOCK_THREAD_CPUTIME_ID } clockid_t;
-#define CLOCK_REALTIME CLOCK_REALTIME
-#define CLOCK_THREAD_CPUTIME_ID CLOCK_THREAD_CPUTIME_ID
+
+#ifndef CLOCK_REALTIME
+typedef int clockid_t;
+# define CLOCK_REALTIME 0
+#endif
+#ifndef CLOCK_THREAD_CPUTIME_ID
+# define CLOCK_THREAD_CPUTIME_ID 106 /* unlikely to clash */
+#endif
 typedef struct
 {
   clockid_t clockid;
diff --git a/nt/mingw-cfg.site b/nt/mingw-cfg.site
index d9a824008c..25d5a40eb1 100644
--- a/nt/mingw-cfg.site
+++ b/nt/mingw-cfg.site
@@ -132,5 +132,7 @@ gl_cv_func_svid_putenv=yes
 ac_cv_func_sbrk=yes
 ac_cv_func_getrlimit=yes
 ac_cv_func_setrlimit=yes
+# Implemented in w32proc.c
+ac_cv_func_timer_settime=yes
 # GCC warnings that produce too much noise
 gl_cv_warn_c__Wredundant_decls=no
diff --git a/src/w32proc.c b/src/w32proc.c
index 23869768cf..20b7d6a220 100644
--- a/src/w32proc.c
+++ b/src/w32proc.c
@@ -651,7 +651,6 @@ timer_gettime (timer_t timerid, struct itimerspec *value)
   ULONGLONG expire, reload;
   CRITICAL_SECTION *crit;
   struct itimer_data *itimer;
-  HANDLE thread_hand;
   bool realtime = timerid.clockid == CLOCK_REALTIME;
 
   if (disable_itimers)
@@ -671,7 +670,6 @@ timer_gettime (timer_t timerid, struct itimerspec *value)
 
   itimer = realtime ? &real_itimer : &prof_itimer;
 
-  ticks_now = w32_get_timer_time (thread_hand);
   t_expire = &itimer->expire;
   t_reload = &itimer->reload;
   crit = realtime ? &crit_real : &crit_prof;
@@ -753,15 +751,15 @@ timer_settime (timer_t timerid, int flags,
   else
     reload += ns / (1000000000 / TIMER_TICKS_PER_SEC);
 
-  expire = ex.tv_sec * TIMER_TICKS_PER_SEC;
-  ns = ex.tv_nsec;
-  if (ex.tv_sec == 0
+  expire = value->it_value.tv_sec * TIMER_TICKS_PER_SEC;
+  ns = value->it_value.tv_nsec;
+  if (value->it_value.tv_sec == 0
       && 0 < ns && ns < clocks_min * (1000000000 / TIMER_TICKS_PER_SEC))
     expire = clocks_min;
   else
     reload += ns / (1000000000 / TIMER_TICKS_PER_SEC);
 
-  if (flags & TIMER_ABSTIME)
+  if (! (flags & TIMER_ABSTIME))
     expire += ticks_now;
 
   EnterCriticalSection (crit);
-- 
2.17.1

bug#32746: setitimer API is obsolescent

[Lars Ingebrigtsen]

Paul Eggert <eggert@cs.ucla.edu> writes:

> Eli Zaretskii wrote:
>
>> However, I think it might be too early to remove support for
>> setitimer, because some platforms have problems with it.  AFAIK, macOS
>> and some *BSD version either don't have that family or have buggy
>> implementations.
>
> In that case never mind about removing use of setitimer.

OK; it sounds like the decision was to not do this change any time soon,
so I'm closing this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no