Subject: bug#32746: stop using obsolescent setitimer API

From: Eli Zaretskii <[email-masked]>
Date: Mon, 17 Sep 2018 18:01:56 +0300

> From: Paul Eggert <[email-masked]>
> Date: Sun, 16 Sep 2018 13:19:45 -0700
> 
> POSIX obsoleted the getitimer/setitimer API a decade ago, and now's a good time 
> for Emacs to stop using this obsolescent API, partly to simplify other timestamp 
> improvements that I have in mind, partly to head off any other lurking bugs like 
> the problem I just fixed in commit 238c7cd730819ddba2dbde3c46ee36136575695b 
> relating to a mistaken assumption about the old API.
> 
> Attached please find two proposed patches. The first simplifies Emacs proper to 
> just use the current timer_gettime/timer_settime API instead (this is the 
> standard replacement for the obsolescent API). The second patch affects only the 
> MS-Windows code, modifying it to use the new API instead of the old one; I 
> haven't tested it as I don't use MS-Windows.

Thanks, I have a couple of comments/questions about the MS-Windows
changes (thanks for working on that):

> +struct sigevent
>  {
> -  struct  timeval it_interval;	/* timer interval */
> -  struct  timeval it_value;	/* current value */
> +  int sigev_notify;
> +  int sigev_signo;
> +  union sigval sigev_value;
> +  void (*sigev_notify_function) (union sigval);
> +  pthread_attr_t *sigev_notify_attributes;

I'd prefer not to use pthread_attr_t, as that could clash with some
MinGW header.

> +typedef enum { CLOCK_REALTIME, CLOCK_THREAD_CPUTIME_ID } clockid_t;

Some versions of MinGW have clockid_t (for clock_gettime and
clock_settime), so this might cause conflicts.

> +#define CLOCK_REALTIME CLOCK_REALTIME
> +#define CLOCK_THREAD_CPUTIME_ID CLOCK_THREAD_CPUTIME_ID

Some of these are declared/defined in MinGW headers in some versions.

> +  HANDLE thread_hand;

This seems to be unused, and is probably related to the other issues
below.

> -  itimer = (which == ITIMER_REAL) ? &real_itimer : &prof_itimer;
> +  itimer = realtime ? &real_itimer : &prof_itimer;
>  
> -  ticks_now = w32_get_timer_time ((which == ITIMER_REAL)
> +  ticks_now = w32_get_timer_time (realtime
>  				  ? NULL
>  				  : GetCurrentThread ());
>  
> +  itimer = realtime ? &real_itimer : &prof_itimer;
> +
> +  ticks_now = w32_get_timer_time (thread_hand);

This seems to do the same stuff twicer, using thread_hand that is
uninitialized.

> -  expire = value->it_value.tv_sec * TIMER_TICKS_PER_SEC;
> -  usecs = value->it_value.tv_usec;
> -  if (value->it_value.tv_sec == 0
> -      && usecs * TIMER_TICKS_PER_SEC < clocks_min * 1000000)
> +  expire = ex.tv_sec * TIMER_TICKS_PER_SEC;
> +  ns = ex.tv_nsec;
> +  if (ex.tv_sec == 0
> +      && 0 < ns && ns < clocks_min * (1000000000 / TIMER_TICKS_PER_SEC))
>      expire = clocks_min;

This seems to use an undefined variable 'ex'.

> -  expire += ticks_now;
> +  if (flags & TIMER_ABSTIME)
> +    expire += ticks_now;

I don't think I understand the reason for the condition you added.

> +#ifdef HAVE_ITIMERSPEC

If we will have a test for itimerspec, we will probably need to edit
nt/mingw-cfg.site to keep the configure script happy.

Thanks again for working on this.