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.
prev parent 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).