Two related changes to timing code:

First, make absolute times unsigned.  This means that it's safe to
depend on their overflow behaviour (which is undefined for signed
integers).  This requires a little extra care in handling comparisons,
but I think I've correctly adjusted them all.

Second, functions registered with schedule_timer() are guaranteed to be
called with precisely the time that was returned by schedule_timer().
Thus, it's only necessary to check these values for equality rather than
doing risky range checks, so do that.

The timing code still does lots that's undefined, unnecessary, or just
wrong, but this is a good start.

[originally from svn r9667]

diff --git a/notiming.c b/notiming.c
index 384fa670a8f7d02df45cc958c9613dfd9f8b033c..5fe440389e0b95f6a191a7faf4dcd69e9dbbe319 100644 (file)
--- a/notiming.c
+++ b/notiming.c
@@ -11,7 +11,7 @@
 
 #include "putty.h"
 
-long schedule_timer(int ticks, timer_fn_t fn, void *ctx)
+unsigned long schedule_timer(int ticks, timer_fn_t fn, void *ctx)
 {
     return 0;
 }

diff --git a/pinger.c b/pinger.c
index 3e2626f562b3e9386067d613b249f8860425dc09..3f533ae6e0868c3721dda70ecb5b0b7cb8618e89 100644 (file)
--- a/pinger.c
+++ b/pinger.c
@@ -8,18 +8,18 @@
 struct pinger_tag {
     int interval;
     int pending;
-    long next;
+    unsigned long next;
     Backend *back;
     void *backhandle;
 };
 
 static void pinger_schedule(Pinger pinger);
 
-static void pinger_timer(void *ctx, long now)
+static void pinger_timer(void *ctx, unsigned long now)
 {
     Pinger pinger = (Pinger)ctx;
 
-    if (pinger->pending && now - pinger->next >= 0) {
+    if (pinger->pending && now == pinger->next) {
        pinger->back->special(pinger->backhandle, TS_PING);
        pinger->pending = FALSE;
        pinger_schedule(pinger);

diff --git a/putty.h b/putty.h
index bc400fb2fdde94e01cf433ef9816d3741a5f2424..6534da7403f3f83bb059f99b0488ff4249212f2b 100644 (file)
--- a/putty.h
+++ b/putty.h
@@ -1383,11 +1383,11 @@ char *get_random_data(int bytes);      /* used in cmdgen.c */
  * GETTICKCOUNT() and compare the result with the returned `next'
  * value to find out how long you have to make your next wait().)
  */
-typedef void (*timer_fn_t)(void *ctx, long now);
-long schedule_timer(int ticks, timer_fn_t fn, void *ctx);
+typedef void (*timer_fn_t)(void *ctx, unsigned long now);
+unsigned long schedule_timer(int ticks, timer_fn_t fn, void *ctx);
 void expire_timer_context(void *ctx);
-int run_timers(long now, long *next);
-void timer_change_notify(long next);
+int run_timers(unsigned long now, unsigned long *next);
+void timer_change_notify(unsigned long next);
 
 /*
  * Define no-op macros for the jump list functions, on platforms that

diff --git a/ssh.c b/ssh.c
index 4bbb30bd08d5afdc4d0d17f9111fa1cd08f33024..1cbb471579619d669eaddcaa93f8e9bf3cb60a0c 100644 (file)
--- a/ssh.c
+++ b/ssh.c
@@ -772,7 +772,7 @@ static int ssh_do_close(Ssh ssh, int notify_exit);
 static unsigned long ssh_pkt_getuint32(struct Packet *pkt);
 static int ssh2_pkt_getbool(struct Packet *pkt);
 static void ssh_pkt_getstring(struct Packet *pkt, char **p, int *length);
-static void ssh2_timer(void *ctx, long now);
+static void ssh2_timer(void *ctx, unsigned long now);
 static void do_ssh2_transport(Ssh ssh, void *vin, int inlen,
                              struct Packet *pktin);
 static void ssh2_msg_unexpected(Ssh ssh, struct Packet *pktin);
@@ -981,7 +981,7 @@ struct ssh_tag {
     unsigned long incoming_data_size, outgoing_data_size, deferred_data_size;
     unsigned long max_data_size;
     int kex_in_progress;
-    long next_rekey, last_rekey;
+    unsigned long next_rekey, last_rekey;
     char *deferred_rekey_reason;    /* points to STATIC string; don't free */
 
     /*
@@ -9482,7 +9482,7 @@ static void ssh2_protocol_setup(Ssh ssh)
     ssh->packet_dispatch[SSH2_MSG_DEBUG] = ssh2_msg_debug;
 }
 
-static void ssh2_timer(void *ctx, long now)
+static void ssh2_timer(void *ctx, unsigned long now)
 {
     Ssh ssh = (Ssh)ctx;
 
@@ -9490,7 +9490,7 @@ static void ssh2_timer(void *ctx, long now)
        return;
 
     if (!ssh->kex_in_progress && conf_get_int(ssh->conf, CONF_ssh_rekey_time) != 0 &&
-       now - ssh->next_rekey >= 0) {
+       now == ssh->next_rekey) {
        do_ssh2_transport(ssh, "timeout", -1, NULL);
     }
 }
@@ -9773,10 +9773,10 @@ static void ssh_reconfig(void *handle, Conf *conf)
     rekey_time = conf_get_int(conf, CONF_ssh_rekey_time);
     if (conf_get_int(ssh->conf, CONF_ssh_rekey_time) != rekey_time &&
        rekey_time != 0) {
-       long new_next = ssh->last_rekey + rekey_time*60*TICKSPERSEC;
-       long now = GETTICKCOUNT();
+       unsigned long new_next = ssh->last_rekey + rekey_time*60*TICKSPERSEC;
+       unsigned long now = GETTICKCOUNT();
 
-       if (new_next - now < 0) {
+       if (now - ssh->last_rekey > rekey_time*60*TICKSPERSEC) {
            rekeying = "timeout shortened";
        } else {
            ssh->next_rekey = schedule_timer(new_next - now, ssh2_timer, ssh);

diff --git a/sshrand.c b/sshrand.c
index b728fd95f9268c78c3bd61313bd52279cbdfd61e..4c33f4a01877e488738ebe5297b8beb0302df3d4 100644 (file)
--- a/sshrand.c
+++ b/sshrand.c
@@ -199,9 +199,9 @@ static void random_add_heavynoise_bitbybit(void *noise, int length)
     pool.poolpos = i;
 }
 
-static void random_timer(void *ctx, long now)
+static void random_timer(void *ctx, unsigned long now)
 {
-    if (random_active > 0 && now - next_noise_collection >= 0) {
+    if (random_active > 0 && now == next_noise_collection) {
        noise_regular();
        next_noise_collection =
            schedule_timer(NOISE_REGULAR_INTERVAL, random_timer, &pool);

diff --git a/terminal.c b/terminal.c
index cc4b0d8f91862df48be0141cf93ce4009bfd3530..72bfbc9b12f867370a750a2ce1824ee73fd1e523 100644 (file)
--- a/terminal.c
+++ b/terminal.c
@@ -1065,32 +1065,32 @@ static termline *lineptr(Terminal *term, int y, int lineno, int screen)
 static void term_schedule_tblink(Terminal *term);
 static void term_schedule_cblink(Terminal *term);
 
-static void term_timer(void *ctx, long now)
+static void term_timer(void *ctx, unsigned long now)
 {
     Terminal *term = (Terminal *)ctx;
     int update = FALSE;
 
-    if (term->tblink_pending && now - term->next_tblink >= 0) {
+    if (term->tblink_pending && now == term->next_tblink) {
        term->tblinker = !term->tblinker;
        term->tblink_pending = FALSE;
        term_schedule_tblink(term);
        update = TRUE;
     }
 
-    if (term->cblink_pending && now - term->next_cblink >= 0) {
+    if (term->cblink_pending && now == term->next_cblink) {
        term->cblinker = !term->cblinker;
        term->cblink_pending = FALSE;
        term_schedule_cblink(term);
        update = TRUE;
     }
 
-    if (term->in_vbell && now - term->vbell_end >= 0) {
+    if (term->in_vbell && now == term->vbell_end) {
        term->in_vbell = FALSE;
        update = TRUE;
     }
 
     if (update ||
-       (term->window_update_pending && now - term->next_update >= 0))
+       (term->window_update_pending && now == term->next_update))
        term_update(term);
 }
 

diff --git a/timing.c b/timing.c
index ffea4e148e1f7bfa612d4786bb56ca94edd9c128..11a0305000b94ba753f31a8afee75445172c1a43 100644 (file)
--- a/timing.c
+++ b/timing.c
@@ -36,13 +36,13 @@
 struct timer {
     timer_fn_t fn;
     void *ctx;
-    long now;
-    long when_set;
+    unsigned long now;
+    unsigned long when_set;
 };
 
 static tree234 *timers = NULL;
 static tree234 *timer_contexts = NULL;
-static long now = 0L;
+static unsigned long now = 0L;
 
 static int compare_timers(void *av, void *bv)
 {
@@ -106,9 +106,9 @@ static void init_timers(void)
     }
 }
 
-long schedule_timer(int ticks, timer_fn_t fn, void *ctx)
+unsigned long schedule_timer(int ticks, timer_fn_t fn, void *ctx)
 {
-    long when;
+    unsigned long when;
     struct timer *t, *first;
 
     init_timers();
@@ -153,7 +153,7 @@ long schedule_timer(int ticks, timer_fn_t fn, void *ctx)
  * Returns the time (in ticks) expected until the next timer after
  * that triggers.
  */
-int run_timers(long anow, long *next)
+int run_timers(unsigned long anow, unsigned long *next)
 {
     struct timer *first;
 
@@ -174,8 +174,8 @@ int run_timers(long anow, long *next)
             */
            delpos234(timers, 0);
            sfree(first);
-       } else if (first->now - now <= 0 ||
-                  now - (first->when_set - 10) < 0) {
+       } else if (now - first->when_set - 10 >
+                  first->now - first->when_set - 10) {
            /*
             * This timer is active and has reached its running
             * time. Run it.

diff --git a/unix/gtkwin.c b/unix/gtkwin.c
index 241aa6bcdd9b159857594a7a64182fe99d08f3e3..2c0df9cb220d8910324c5c8e2d5329d27d3aa7ac 100644 (file)
--- a/unix/gtkwin.c
+++ b/unix/gtkwin.c
@@ -1400,13 +1400,18 @@ void notify_remote_exit(void *frontend)
 
 static gint timer_trigger(gpointer data)
 {
-    long now = GPOINTER_TO_LONG(data);
-    long next;
+    unsigned long now = GPOINTER_TO_LONG(data);
+    unsigned long next, then;
     long ticks;
 
     if (run_timers(now, &next)) {
-       ticks = next - GETTICKCOUNT();
-       timer_id = gtk_timeout_add(ticks > 0 ? ticks : 1, timer_trigger,
+       then = now;
+       now = GETTICKCOUNT();
+       if (now - then > next - then)
+           ticks = 0;
+       else
+           ticks = next - now;
+       timer_id = gtk_timeout_add(ticks, timer_trigger,
                                   LONG_TO_GPOINTER(next));
     }
 
@@ -1417,7 +1422,7 @@ static gint timer_trigger(gpointer data)
     return FALSE;
 }
 
-void timer_change_notify(long next)
+void timer_change_notify(unsigned long next)
 {
     long ticks;
 

diff --git a/unix/uxcons.c b/unix/uxcons.c
index 73126c021b5024cf475aef8ed99d4ebbf309f472..882d2c9bfe4fa949b704f2ec1a1fc081681c476e 100644 (file)
--- a/unix/uxcons.c
+++ b/unix/uxcons.c
@@ -70,7 +70,7 @@ void notify_remote_exit(void *frontend)
 {
 }
 
-void timer_change_notify(long next)
+void timer_change_notify(unsigned long next)
 {
 }
 

diff --git a/unix/uxplink.c b/unix/uxplink.c
index 61b9426bb8823e2a5534cba9e4975aab62e5cadc..0d89622530dbb6819485f939581c86d43ab8a816 100644 (file)
--- a/unix/uxplink.c
+++ b/unix/uxplink.c
@@ -593,7 +593,7 @@ int main(int argc, char **argv)
     int errors;
     int use_subsystem = 0;
     int got_host = FALSE;
-    long now;
+    unsigned long now;
     struct winsize size;
 
     fdlist = NULL;
@@ -1016,12 +1016,17 @@ int main(int argc, char **argv)
        }
 
        do {
-           long next, ticks;
+           unsigned long next, then;
+           long ticks;
            struct timeval tv, *ptv;
 
            if (run_timers(now, &next)) {
-               ticks = next - GETTICKCOUNT();
-               if (ticks < 0) ticks = 0;   /* just in case */
+               then = now;
+               now = GETTICKCOUNT();
+               if (now - then > next - then)
+                   ticks = 0;
+               else
+                   ticks = next - now;
                tv.tv_sec = ticks / 1000;
                tv.tv_usec = ticks % 1000 * 1000;
                ptv = &tv;

diff --git a/unix/uxsftp.c b/unix/uxsftp.c
index a92cfc9d30760446f438d6ac8f1d7d6fac5a121c..f68685a01c32c414e0d42706c05e7d57888416c5 100644 (file)
--- a/unix/uxsftp.c
+++ b/unix/uxsftp.c
@@ -443,7 +443,7 @@ static int ssh_sftp_do_select(int include_stdin, int no_fds_ok)
     fd_set rset, wset, xset;
     int i, fdcount, fdsize, *fdlist;
     int fd, fdstate, rwx, ret, maxfd;
-    long now = GETTICKCOUNT();
+    unsigned long now = GETTICKCOUNT();
 
     fdlist = NULL;
     fdcount = fdsize = 0;
@@ -489,13 +489,17 @@ static int ssh_sftp_do_select(int include_stdin, int no_fds_ok)
            FD_SET_MAX(0, maxfd, rset);
 
        do {
-           long next, ticks;
+           unsigned long next, then;
+           long ticks;
            struct timeval tv, *ptv;
 
            if (run_timers(now, &next)) {
-               ticks = next - GETTICKCOUNT();
-               if (ticks <= 0)
-                   ticks = 1;         /* just in case */
+               then = now;
+               now = GETTICKCOUNT();
+               if (now - then > next - then)
+                   ticks = 0;
+               else
+                   ticks = next - now;
                tv.tv_sec = ticks / 1000;
                tv.tv_usec = ticks % 1000 * 1000;
                ptv = &tv;

diff --git a/windows/wincons.c b/windows/wincons.c
index 9824a5a5b7c82320f654e8800217d3dbf2e44c54..508be3f8d522855931d643e9542eca40427bb72d 100644 (file)
--- a/windows/wincons.c
+++ b/windows/wincons.c
@@ -41,7 +41,7 @@ void notify_remote_exit(void *frontend)
 {
 }
 
-void timer_change_notify(long next)
+void timer_change_notify(unsigned long next)
 {
 }
 

diff --git a/windows/window.c b/windows/window.c
index d970dd1950b4102f59516f340918b17cdc51d568..5fe21787e10c34a3a9cdd5bc47dcd598c75643af 100644 (file)
--- a/windows/window.c
+++ b/windows/window.c
@@ -2012,10 +2012,14 @@ void notify_remote_exit(void *fe)
     }
 }
 
-void timer_change_notify(long next)
+void timer_change_notify(unsigned long next)
 {
-    long ticks = next - GETTICKCOUNT();
-    if (ticks <= 0) ticks = 1;        /* just in case */
+    unsigned long now = GETTICKCOUNT();
+    long ticks;
+    if (now - next < INT_MAX)
+       ticks = 0;
+    else
+       ticks = next - now;
     KillTimer(hwnd, TIMING_TIMER_ID);
     SetTimer(hwnd, TIMING_TIMER_ID, ticks, NULL);
     timing_next_time = next;
@@ -2042,7 +2046,7 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message,
     switch (message) {
       case WM_TIMER:
        if ((UINT_PTR)wParam == TIMING_TIMER_ID) {
-           long next;
+           unsigned long next;
 
            KillTimer(hwnd, TIMING_TIMER_ID);
            if (run_timers(timing_next_time, &next)) {
@@ -5354,9 +5358,9 @@ static int flashing = 0;
  * Timer for platforms where we must maintain window flashing manually
  * (e.g., Win95).
  */
-static void flash_window_timer(void *ctx, long now)
+static void flash_window_timer(void *ctx, unsigned long now)
 {
-    if (flashing && now - next_flash >= 0) {
+    if (flashing && now == next_flash) {
        flash_window(1);
     }
 }

diff --git a/windows/winplink.c b/windows/winplink.c
index 58063352745dbaf7a3e98d779c6ce6f79fa5bc14..7ec98e7fd5bf9e75572d081c7fdada8946d1201a 100644 (file)
--- a/windows/winplink.c
+++ b/windows/winplink.c
@@ -289,7 +289,7 @@ int main(int argc, char **argv)
     int errors;
     int got_host = FALSE;
     int use_subsystem = 0;
-    long now, next;
+    unsigned long now, next, then;
 
     sklist = NULL;
     skcount = sksize = 0;
@@ -634,8 +634,12 @@ int main(int argc, char **argv)
        }
 
        if (run_timers(now, &next)) {
-           ticks = next - GETTICKCOUNT();
-           if (ticks < 0) ticks = 0;  /* just in case */
+       then = now;
+       now = GETTICKCOUNT();
+       if (now - then > next - then)
+           ticks = 0;
+       else
+           ticks = next - now;
        } else {
            ticks = INFINITE;
        }

diff --git a/windows/winser.c b/windows/winser.c
index e0fc20e7f8988ba2332a9766e7ccd9556b8c050f..086d3e5cd4fcb2b3e7fbe8b767ac36748b0afd38 100644 (file)
--- a/windows/winser.c
+++ b/windows/winser.c
@@ -331,11 +331,11 @@ static void serial_size(void *handle, int width, int height)
     return;
 }
 
-static void serbreak_timer(void *ctx, long now)
+static void serbreak_timer(void *ctx, unsigned long now)
 {
     Serial serial = (Serial)ctx;
 
-    if (now >= serial->clearbreak_time && serial->port) {
+    if (now == serial->clearbreak_time && serial->port) {
        ClearCommBreak(serial->port);
        serial->break_in_progress = FALSE;
        logevent(serial->frontend, "Finished serial break");

diff --git a/windows/winsftp.c b/windows/winsftp.c
index e6b55c1dd82e047e71da0a2d743ff98e6363c881..d4c5fa6edbbdedde3c1bffff1249cd8ae0bf62ba 100644 (file)
--- a/windows/winsftp.c
+++ b/windows/winsftp.c
@@ -486,15 +486,20 @@ extern int select_result(WPARAM, LPARAM);
 int do_eventsel_loop(HANDLE other_event)
 {
     int n, nhandles, nallhandles, netindex, otherindex;
-    long next, ticks;
+    unsigned long next, then;
+    long ticks;
     HANDLE *handles;
     SOCKET *sklist;
     int skcount;
-    long now = GETTICKCOUNT();
+    unsigned long now = GETTICKCOUNT();
 
     if (run_timers(now, &next)) {
-       ticks = next - GETTICKCOUNT();
-       if (ticks < 0) ticks = 0;  /* just in case */
+       then = now;
+       now = GETTICKCOUNT();
+       if (now - then > next - then)
+           ticks = 0;
+       else
+           ticks = next - now;
     } else {
        ticks = INFINITE;
     }