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.

git-svn-id: svn://svn.tartarus.org/sgt/putty@9667 cda61777-01e9-0310-a592-d414129be87e

diff --git a/notiming.c b/notiming.c
index 384fa67..5fe4403 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 3e2626f..3f533ae 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 bc400fb..6534da7 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 4bbb30b..1cbb471 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 b728fd9..4c33f4a 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 cc4b0d8..72bfbc9 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 ffea4e1..11a0305 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 241aa6b..2c0df9c 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 73126c0..882d2c9 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 61b9426..0d89622 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 a92cfc9..f68685a 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 9824a5a..508be3f 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 d970dd1..5fe2178 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 5806335..7ec98e7 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 e0fc20e..086d3e5 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 e6b55c1..d4c5fa6 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;
     }