Revise scrotWaitUntil interface

src/options.c

@@ -390,7 +390,8 @@ void optionsParse(int argc, char *argv[])
             opt.delaySelection = *optarg == 'b';
             if (opt.delaySelection)
                 ++optarg;
-            opt.delay = optionsParseNum(optarg, 0, INT_MAX, &errmsg);
+            /* NOTE: div 1000 so that converting to milliseconds doesn't overflow */
+            opt.delay = optionsParseNum(optarg, 0, INT_MAX/1000, &errmsg);
             if (errmsg) {
                 errx(EXIT_FAILURE, "option --delay: '%s' is %s", optarg,
                     errmsg);

src/scrot.c

@@ -74,7 +74,7 @@ static size_t scrotHaveFileExtension(const char *, char **);
 static Imlib_Image scrotGrabFocused(void);
 static void applyFilterIfRequired(void);
 static Imlib_Image scrotGrabAutoselect(void);
-static void scrotWaitUntil(const struct timespec *);
+static long miliToNanoSec(int);
 static Imlib_Image scrotGrabShotMulti(void);
 static Imlib_Image scrotGrabShotMonitor(void);
 static Imlib_Image scrotGrabStackWindows(void);
@@ -108,7 +108,7 @@ int main(int argc, char *argv[])
     struct tm *tm;
 
     /* Get the time ASAP to reduce the timing error in case --delay is used. */
-    clock_gettime(CONTINUOUS_CLOCK, &opt.delayStart);
+    opt.delayStart = clockNow();
 
     atexit(uninitXAndImlib);
 
@@ -145,7 +145,7 @@ int main(int argc, char *argv[])
      * screenshot-taking above or it will skew the timing.
      */
     clock_gettime(CLOCK_REALTIME, &timeStamp);
-    if (timeStamp.tv_nsec >= 500*1000*1000) {
+    if (timeStamp.tv_nsec >= miliToNanoSec(500)) {
         /* Round the timestamp to the nearest second. */
         timeStamp.tv_sec++;
     }
@@ -335,38 +335,67 @@ void scrotDoDelay(void)
         dprintf(STDERR_FILENO, "Taking shot in ");
         for (int i = opt.delay; i > 0; i--) {
             dprintf(STDERR_FILENO, "%d.. ", i);
-            opt.delayStart.tv_sec++;
-            scrotWaitUntil(&opt.delayStart);
+            opt.delayStart = scrotSleepFor(opt.delayStart, 1000);
         }
         dprintf(STDERR_FILENO, "0.\n");
     } else {
-        opt.delayStart.tv_sec += opt.delay;
-        scrotWaitUntil(&opt.delayStart);
+        scrotSleepFor(opt.delayStart, opt.delay * 1000);
     }
 }
 
-/* scrotWaitUntil: clock_nanosleep with a simpler interface and no EINTR nagging
+static long miliToNanoSec(int ms)
+{
+    return ms * 1000L * 1000L;
+}
+
+/* clockNow() has the exact same semantics as CLOCK_MONOTONIC. Except that on
+ * Linux, CLOCK_MONOTONIC does not progress while the system is suspended, so
+ * the non-standard CLOCK_BOOTTIME is used instead to avoid this bug.
  */
-static void scrotWaitUntil(const struct timespec *time)
+struct timespec clockNow(void)
 {
-    /* OpenBSD and OS X lack clock_nanosleep(), so we use a trivial algorithm to
-     * correct for drift and call nanosleep() everywhere.
-     */
+    struct timespec ret;
+#if defined(__linux__)
+    clock_gettime(CLOCK_BOOTTIME, &ret);
+#else
+    clock_gettime(CLOCK_MONOTONIC, &ret);
+#endif
+    return ret;
+}
+
+/* OpenBSD and OS X lack clock_nanosleep(), so we call nanosleep() and use a
+ * trivial algorithm to correct for drift. The end timespec is returned for
+ * callers that want it. EINTR is also dealt with.
+ */
+struct timespec scrotSleepFor(struct timespec start, int ms)
+{
+    scrotAssert(ms >= 0);
+    struct timespec end = {
+        .tv_sec  = start.tv_sec  + (ms / 1000),
+        .tv_nsec = start.tv_nsec + miliToNanoSec(ms % 1000),
+    };
+    if (end.tv_nsec >= miliToNanoSec(1000)) {
+        ++end.tv_sec;
+        end.tv_nsec -= miliToNanoSec(1000);
+    }
+
     struct timespec tmp;
     do {
-        clock_gettime(CONTINUOUS_CLOCK, &tmp);
+        tmp = clockNow();
 
         /* XXX: Use timespecsub(). OS X doesn't have that BSD macro, and libbsd
          * doesn't support OS X save for an unmaintained fork. libobsd supports
          * OS X but doesn't have the macro yet.
          */
-        tmp.tv_sec = time->tv_sec - tmp.tv_sec;
-        tmp.tv_nsec = time->tv_nsec - tmp.tv_nsec;
+        tmp.tv_sec  = end.tv_sec  - tmp.tv_sec;
+        tmp.tv_nsec = end.tv_nsec - tmp.tv_nsec;
         if (tmp.tv_nsec < 0) {
-            tmp.tv_sec--;
-            tmp.tv_nsec += 1000000000L;
+            --tmp.tv_sec;
+            tmp.tv_nsec += miliToNanoSec(1000);
         }
-    } while (nanosleep(&tmp, NULL) == -1 && errno == EINTR);
+    } while (nanosleep(&tmp, NULL) < 0 && errno == EINTR);
+
+    return end;
 }
 
 /* Clip rectangle nicely */
@@ -429,8 +458,7 @@ int scrotGetGeometry(Window target, int *rx, int *ry, int *rw, int *rh)
             /* HACK: there doesn't seem to be any way to figure out whether the
              * raise request was accepted or rejected. so just sleep a bit to
              * give the WM some time to update. */
-            struct timespec t = { .tv_nsec = 160 * 1000L * 1000L };
-            while (nanosleep(&t, &t) < 0 && errno == EINTR);
+            scrotSleepFor(clockNow(), 160);
         }
     }
     stat = XGetWindowAttributes(disp, target, &attr);

src/scrot.h

@@ -28,6 +28,8 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #pragma once
 
+#include <time.h>
+
 #include <X11/Xlib.h>
 #include <Imlib2.h>
 
@@ -39,5 +41,7 @@ extern Screen *scr;
 Window scrotGetWindow(Display *, Window, int, int);
 int scrotGetGeometry(Window, int *, int *, int *, int *);
 void scrotNiceClip(int *, int *, int *, int *);
+struct timespec clockNow(void);
+struct timespec scrotSleepFor(struct timespec, int);
 void scrotDoDelay(void);
 void scrotGrabMousePointer(const Imlib_Image, const int, const int);

src/scrot_selection.c

@@ -233,9 +233,9 @@ bool scrotSelectionGetUserSel(struct SelectionRect *selectionRect)
     ret = XGrabKeyboard(disp, root, False, GrabModeAsync, GrabModeAsync, CurrentTime);
     if (ret == AlreadyGrabbed) {
         int attempts = 20;
-        struct timespec delay = { 0, 50 * 1000L * 1000L };
+        struct timespec t = clockNow();
         do {
-            nanosleep(&delay, NULL);
+            t = scrotSleepFor(t, 50);
             ret = XGrabKeyboard(disp, root, False, GrabModeAsync, GrabModeAsync, CurrentTime);
         } while (--attempts > 0 && ret == AlreadyGrabbed);
     }
@@ -433,7 +433,7 @@ Imlib_Image scrotSelectionSelectMode(void)
             return NULL;
 
     if (!opt.delaySelection) {
-        clock_gettime(CONTINUOUS_CLOCK, &opt.delayStart);
+        opt.delayStart = clockNow();
         scrotDoDelay();
     }
 

src/selection_edge.c

@@ -143,8 +143,7 @@ void selectionEdgeDestroy(void)
          * might not have been updated. a compositor might also buffer frames
          * adding latency. so wait a bit for the screen to update and the
          * selection borders to go away. */
-        struct timespec t = { .tv_nsec = 80 * 1000L * 1000L };
-        while (nanosleep(&t, &t) < 0 && errno == EINTR);
+        scrotSleepFor(clockNow(), 80);
 
         XFree(pe->classHint);
     }

src/util.h

@@ -26,21 +26,9 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #pragma once
 
-#include <time.h>
 #include <stdio.h>
 #include <stdlib.h>
 
-/* On Linux, CLOCK_MONOTONIC does not progress while the system is suspended,
- * and an alternative non-standard clock which does not suffer from this problem
- * called CLOCK_BOOTTIME is available. Scrot's CONTINUOUS_CLOCK has the exact
- * same semantics as CLOCK_MONOTONIC, only it avoids this bug.
- */
-#if defined(__linux__)
-    #define CONTINUOUS_CLOCK CLOCK_BOOTTIME
-#else
-    #define CONTINUOUS_CLOCK CLOCK_MONOTONIC
-#endif
-
 #ifdef DEBUG
     #define scrotAssert(X) do { \
         if (!(X)) { \