unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Paul Eggert <eggert@cs.ucla.edu>
Cc: 32746@debbugs.gnu.org
Subject: bug#32746: stop using obsolescent setitimer API
Date: Mon, 17 Sep 2018 18:01:56 +0300	[thread overview]
Message-ID: <83lg80gobf.fsf@gnu.org> (raw)
In-Reply-To: <3c8e16a0-bae6-4ee7-d8e1-5da307ff6e84@cs.ucla.edu> (message from Paul Eggert on Sun, 16 Sep 2018 13:19:45 -0700)

> From: Paul Eggert <eggert@cs.ucla.edu>
> 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.





      reply	other threads:[~2018-09-17 15:01 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-16 20:19 bug#32746: stop using obsolescent setitimer API Paul Eggert
2018-09-17 15:01 ` Eli Zaretskii [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83lg80gobf.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=32746@debbugs.gnu.org \
    --cc=eggert@cs.ucla.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).