On Wed, Aug 18, 2010 at 09:01:31PM -0700, Jamie Zawinski wrote:
> This looks good, thanks! But I think you do need to go around the
> loop multiple times, because there's technically no guarantee that
> the response event we're polling for in XCheckIfEvent will be the
> first event to come in. There might be some event we don't care
> about that causes select() to return, and then the event we need
> comes in some fraction of a second later.
I saw about three ways to easily implement the select.
1. select with the existing counter (the first patch I sent in)
If select times out (and returns zero) it will check one last time
(which shouldn't be necessary, but but no harm either). If select
found fd readable it will loop to check the event, and select again up
to 10 times if none of them timeout. To address your comment, it does
go around the loop multiple times in both cases. I liked this
solution because it was the most compact change and I felt that it was
extremely highly likely that even if there were other messages going
to xscreensaver-command before the expected event there wouldn't be
ten sets (one to trigger each select return), before the event of
interest. On the other hand I do grant that if the xscreensaver isn't
responding it could happen, and it would return before the current ten
second timeout when the xscreensaver might have responded before ten
seconds. The other problem is if xscreensaver really isn't responding
and there are other messages coming at a very low rate, say 9.9
seconds apart, select always restarts with ten seconds and so it could
wait 99 seconds before returning, where the previous code would be
fixed at ten seconds.
2. select and gettimeofday (patch follows)
This wasn't my first choice because it is just more code, and I
thought it was more complicated. By adding gettimeofday it will wait
ten seconds no matter how many messages come in that it isn't looking
for. It's about ten seconds because I didn't bother calculating the
fractions of a second.
3. one second selects and variations
Instead of a ten second select, use ten one second selects. This is
kind of like #1 only it can't go over ten seconds, but then it's
calling select ten times. Like #1 it can also go way under ten
seconds if other messages are coming in. I didn't code this one up.
Feel free to take your pick. Thanks for your quick reply.
>From 873bb17999c7c3384c36611f35171ce02f889853 Mon Sep 17 00:00:00 2001
From: David Fries <da...@fries.net>
Date: Mon, 16 Aug 2010 21:59:31 -0500
Subject: [PATCH] Call select to wait for the reply instead of sleeping.
This version will wait 10 seconds even when receiving messages that it
isn't interested in. The previous version would execute the loop
10 times, so if there were 11 messages before the event of interest
it wouldn't get it even if it in total only waited a fraction of a second.
---
driver/remote.c | 41 +++++++++++++++++++++++++++++++++++++++--
1 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/driver/remote.c b/driver/remote.c
index 1e67c97..c706f6c 100644
--- a/driver/remote.c
+++ b/driver/remote.c
@@ -371,17 +371,54 @@ xscreensaver_command_response (Display *dpy, Window
window,
Bool verbose_p, Bool exiting_p,
char **error_ret)
{
- int sleep_count = 0;
char err[2048];
XEvent event;
Bool got_event = False;
+ const int timeout = 10;
+
+#if defined(HAVE_SELECT)
+ struct timeval start, now;
+ struct timeval tv;
+ int fd = XConnectionNumber(dpy);
+ fd_set rset, rcopy;
+
+#ifdef GETTIMEOFDAY_TWO_ARGS
+ gettimeofday(&start, NULL);
+#else
+ gettimeofday(&start);
+#endif
+ now=start;
+ FD_ZERO(&rset);
+ FD_SET(fd, &rset);
+ /* Wait for the response for ten seconds (fractions aren't important). */
while (!(got_event = XCheckIfEvent(dpy, &event,
&xscreensaver_command_event_p, 0)) &&
- sleep_count++ < 10)
+ now.tv_sec - start.tv_sec < timeout)
+ {
+ tv.tv_sec = timeout - (now.tv_sec - start.tv_sec);
+ tv.tv_usec = 0;
+ rcopy = rset;
+ printf("sleep %d\n", (int)tv.tv_sec);
+ (void) select (fd+1, &rcopy, NULL, NULL, &tv);
+
+#ifdef GETTIMEOFDAY_TWO_ARGS
+ gettimeofday(&now, NULL);
+#else
+ gettimeofday(&now);
+#endif
+ }
+
+#else
+ int sleep_count = 0;
+
+ while (!(got_event = XCheckIfEvent(dpy, &event,
+ &xscreensaver_command_event_p, 0)) &&
+ sleep_count++ < timeout)
{
sleep(1);
}
+#endif
if (!got_event)
{
--
1.7.1
--
David Fries <da...@fries.net>
http://fries.net/~david/ (PGP encryption key available)
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
