- Refactored a lot of timeout code into a few functions in an attempt to make

them all use the same (hopefully correct) logic to make it less error-prone
  and easier to introduce library-wide where it should be used.

CHANGES

@@ -6,6 +6,11 @@
 
                                   Changelog
 
+Daniel S (7 Feb 2008)
+- Refactored a lot of timeout code into a few functions in an attempt to make
+  them all use the same (hopefully correct) logic to make it less error-prone
+  and easier to introduce library-wide where it should be used.
+
 Yang Tse (6 Feb 2008)
 - Fix an issue in strdup replacement function when dealing with absolutely
   huge strings. Only systems without a standard strdup would be affected.

RELEASE-NOTES

@@ -32,6 +32,7 @@ New curl mirrors:
 This release would not have looked like this without help, code, reports and
 advice from friends like these:
 
- Michal Marek, Dmitry Kurochkin, Niklas Angebrand
+ Michal Marek, Dmitry Kurochkin, Niklas Angebrand, Günter Knauf, Yang Tse,
+ Dan Fandrich
 
         Thanks! (and sorry if I forgot to mention someone)

lib/connect.c

@@ -101,6 +101,66 @@ singleipconnect(struct connectdata *conn,
                 long timeout_ms,
                 bool *connected);
 
+/*
+ * Curl_timeleft() returns the amount of milliseconds left allowed for the
+ * transfer/connection. If the value is negative, the timeout time has already
+ * elapsed.
+ *
+ * If 'nowp' is non-NULL, it points to the current time.
+ * 'duringconnect' is FALSE if not during a connect, as then of course the
+ * connect timeout is not taken into account!
+ */
+long Curl_timeleft(struct connectdata *conn,
+                   struct timeval *nowp,
+                   bool duringconnect)
+{
+  struct SessionHandle *data = conn->data;
+  int timeout_set = 0;
+  long timeout_ms = duringconnect?DEFAULT_CONNECT_TIMEOUT:0;
+  struct timeval now;
+
+  /* if a timeout is set, use the most restrictive one */
+
+  if(data->set.timeout > 0)
+    timeout_set |= 1;
+  if(duringconnect && (data->set.connecttimeout > 0))
+    timeout_set |= 2;
+
+  switch (timeout_set) {
+  case 1:
+    timeout_ms = data->set.timeout;
+    break;
+  case 2:
+    timeout_ms = data->set.connecttimeout;
+    break;
+  case 3:
+    if(data->set.timeout < data->set.connecttimeout)
+      timeout_ms = data->set.timeout;
+    else
+      timeout_ms = data->set.connecttimeout;
+    break;
+  default:
+    /* use the default */
+    if(!duringconnect)
+      /* if we're not during connect, there's no default timeout so if we're
+         at zero we better just return zero and not make it a negative number
+         by the math below */
+      return 0;
+    break;
+  }
+
+  if(!nowp) {
+    now = Curl_tvnow();
+    nowp = &now;
+  }
+
+  /* substract elapsed time */
+  timeout_ms -= Curl_tvdiff(*nowp, data->progress.t_startsingle);
+
+  return timeout_ms;
+}
+
+
 /*
  * Curl_nonblock() set the given socket to either blocking or non-blocking
  * mode based on the 'nonblock' boolean argument. This function is highly
@@ -533,42 +593,33 @@ CURLcode Curl_is_connected(struct connectdata *conn,
   CURLcode code = CURLE_OK;
   curl_socket_t sockfd = conn->sock[sockindex];
   long allow = DEFAULT_CONNECT_TIMEOUT;
-  long allow_total = 0;
-  long has_passed;
 
   DEBUGASSERT(sockindex >= FIRSTSOCKET && sockindex <= SECONDARYSOCKET);
 
   *connected = FALSE; /* a very negative world view is best */
 
-  /* Evaluate in milliseconds how much time that has passed */
-  has_passed = Curl_tvdiff(Curl_tvnow(), data->progress.t_startsingle);
-
-  /* subtract the most strict timeout of the ones */
-  if(data->set.timeout && data->set.connecttimeout) {
-    if(data->set.timeout < data->set.connecttimeout)
-      allow_total = allow = data->set.timeout;
-    else
-      allow = data->set.connecttimeout;
-  }
-  else if(data->set.timeout) {
-    allow_total = allow = data->set.timeout;
-  }
-  else if(data->set.connecttimeout) {
-    allow = data->set.connecttimeout;
-  }
-
-  if(has_passed > allow ) {
-    /* time-out, bail out, go home */
-    failf(data, "Connection time-out after %ld ms", has_passed);
-    return CURLE_OPERATION_TIMEDOUT;
-  }
   if(conn->bits.tcpconnect) {
     /* we are connected already! */
+    long allow_total = 0;
+
+    /* subtract the most strict timeout of the ones */
+    if(data->set.timeout)
+      allow_total = data->set.timeout;
+
     Curl_expire(data, allow_total);
     *connected = TRUE;
     return CURLE_OK;
   }
 
+  /* figure out how long time we have left to connect */
+  allow = Curl_timeleft(conn, NULL, TRUE);
+
+  if(allow < 0) {
+    /* time-out, bail out, go home */
+    failf(data, "Connection time-out");
+    return CURLE_OPERATION_TIMEDOUT;
+  }
+
   Curl_expire(data, allow);
 
   /* check for connect without timeout as we want to return immediately */
@@ -821,7 +872,6 @@ CURLcode Curl_connecthost(struct connectdata *conn,  /* context */
   int num_addr;
   Curl_addrinfo *ai;
   Curl_addrinfo *curr_addr;
-  int timeout_set = 0;
 
   struct timeval after;
   struct timeval before = Curl_tvnow();
@@ -834,39 +884,13 @@ CURLcode Curl_connecthost(struct connectdata *conn,  /* context */
 
   *connected = FALSE; /* default to not connected */
 
-  /* if a timeout is set, use the most restrictive one */
+  /* get the timeout left */
+  timeout_ms = Curl_timeleft(conn, &before, TRUE);
 
-  if(data->set.timeout > 0)
+  if(timeout_ms < 0) {
-    timeout_set += 1;
+    /* a precaution, no need to continue if time already is up */
-  if(data->set.connecttimeout > 0)
+    failf(data, "Connection time-out");
-    timeout_set += 2;
+    return CURLE_OPERATION_TIMEDOUT;
-
-  switch (timeout_set) {
-  case 1:
-    timeout_ms = data->set.timeout;
-    break;
-  case 2:
-    timeout_ms = data->set.connecttimeout;
-    break;
-  case 3:
-    if(data->set.timeout < data->set.connecttimeout)
-      timeout_ms = data->set.timeout;
-    else
-      timeout_ms = data->set.connecttimeout;
-    break;
-  default:
-    timeout_ms = DEFAULT_CONNECT_TIMEOUT;
-    break;
-  }
-
-  if(timeout_set > 0) {
-    /* if a timeout was already set, substract elapsed time */
-    timeout_ms -= Curl_tvdiff(before, data->progress.t_startsingle);
-    if(timeout_ms < 0) {
-      /* a precaution, no need to continue if time already is up */
-      failf(data, "Connection time-out");
-      return CURLE_OPERATION_TIMEDOUT;
-    }
   }
   Curl_expire(data, timeout_ms);
 

lib/connect.h

@@ -7,7 +7,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2007, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2008, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -31,14 +31,20 @@ CURLcode Curl_is_connected(struct connectdata *conn,
                            bool *connected);
 
 CURLcode Curl_connecthost(struct connectdata *conn,
-                          const struct Curl_dns_entry *host, /* connect to this */
+                          const struct Curl_dns_entry *host, /* connect to
+                                                                this */
                           curl_socket_t *sockconn, /* not set if error */
                           Curl_addrinfo **addr, /* the one we used */
-                          bool *connected /* truly connected? */
+                          bool *connected); /* truly connected? */
-                          );
 
 CURLcode Curl_store_ip_addr(struct connectdata *conn);
 
+/* generic function that returns how much time there's left to run, according
+   to the timeouts set */
+long Curl_timeleft(struct connectdata *conn,
+                   struct timeval *nowp,
+                   bool duringconnect);
+
 #define DEFAULT_CONNECT_TIMEOUT 300000 /* milliseconds == five minutes */
 
 #endif

lib/ftp.c

@@ -300,43 +300,14 @@ static bool isBadFtpString(const char *string)
  */
 static CURLcode AllowServerConnect(struct connectdata *conn)
 {
-  long timeout_ms;
   struct SessionHandle *data = conn->data;
   curl_socket_t sock = conn->sock[SECONDARYSOCKET];
-  int timeout_set = 0;
+  long timeout_ms = Curl_timeleft(conn, NULL, TRUE);
 
-  /* if a timeout is set, use the most restrictive one */
+  if(timeout_ms < 0) {
-
+    /* if a timeout was already reached, bail out */
-  if(data->set.timeout > 0)
+    failf(data, "Timed out before server could connect to us");
-    timeout_set += 1;
+    return CURLE_OPERATION_TIMEDOUT;
-  if(data->set.connecttimeout > 0)
-    timeout_set += 2;
-
-  switch (timeout_set) {
-  case 1:
-    timeout_ms = data->set.timeout;
-    break;
-  case 2:
-    timeout_ms = data->set.connecttimeout;
-    break;
-  case 3:
-    if(data->set.timeout < data->set.connecttimeout)
-      timeout_ms = data->set.timeout;
-    else
-      timeout_ms = data->set.connecttimeout;
-    break;
-  default:
-    timeout_ms = 60000; /* 60 seconds default timeout */
-    break;
-  }
-
-  if(timeout_set > 0) {
-    /* if a timeout was already set, substract elapsed time */
-    timeout_ms -= Curl_tvdiff(Curl_tvnow(), conn->now);
-    if(timeout_ms < 0) {
-      failf(data, "Timed out before server could connect to us");
-      return CURLE_OPERATION_TIMEDOUT;
-    }
   }
 
   switch (Curl_socket_ready(sock, CURL_SOCKET_BAD, (int)timeout_ms)) {

lib/gtls.c

@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2007, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2008, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -154,23 +154,7 @@ static CURLcode handshake(struct connectdata *conn,
     rc = gnutls_handshake(session);
 
     if((rc == GNUTLS_E_AGAIN) || (rc == GNUTLS_E_INTERRUPTED)) {
-      long timeout_ms = DEFAULT_CONNECT_TIMEOUT;
+      long timeout_ms = Curl_connecttimeleft(conn, NULL, duringconnect);
-      long has_passed;
-
-      if(duringconnect && data->set.connecttimeout)
-        timeout_ms = data->set.connecttimeout;
-
-      if(data->set.timeout) {
-        /* get the strictest timeout of the ones converted to milliseconds */
-        if(data->set.timeout < timeout_ms)
-          timeout_ms = data->set.timeout;
-      }
-
-      /* Evaluate in milliseconds how much time that has passed */
-      has_passed = Curl_tvdiff(Curl_tvnow(), data->progress.t_startsingle);
-
-      /* subtract the passed time */
-      timeout_ms -= has_passed;
 
       if(timeout_ms < 0) {
         /* a precaution, no need to continue if time already is up */

lib/qssl.c

@@ -172,17 +172,8 @@ static CURLcode Curl_qsossl_handshake(struct connectdata * conn, int sockindex)
   if(!data->set.ssl.verifyhost)
     h->exitPgm = Curl_qsossl_trap_cert;
 
-  if(data->set.connecttimeout) {
+  /* figure out how long time we should wait at maximum */
-    timeout_ms = data->set.connecttimeout;
+  timeout_ms = Curl_timeleft(conn, NULL, TRUE);
-
-    if(data->set.timeout)
-      if(timeout_ms > data->set.timeout)
-        timeout_ms = data->set.timeout;
-    }
-  else if(data->set.timeout)
-    timeout_ms = data->set.timeout;
-  else
-    timeout_ms = DEFAULT_CONNECT_TIMEOUT;
 
   /* SSL_Handshake() timeout resolution is second, so round up. */
 

lib/socks.c

@@ -138,18 +138,7 @@ CURLcode Curl_SOCKS4(const char *proxy_name,
   struct SessionHandle *data = conn->data;
 
   /* get timeout */
-  if(data->set.timeout && data->set.connecttimeout) {
+  timeout = Curl_timeleft(conn, NULL, TRUE);
-    if(data->set.timeout < data->set.connecttimeout)
-      timeout = data->set.timeout;
-    else
-      timeout = data->set.connecttimeout;
-  }
-  else if(data->set.timeout)
-    timeout = data->set.timeout;
-  else if(data->set.connecttimeout)
-    timeout = data->set.connecttimeout;
-  else
-    timeout = DEFAULT_CONNECT_TIMEOUT;
 
   Curl_nonblock(sock, FALSE);
 
@@ -403,18 +392,7 @@ CURLcode Curl_SOCKS5(const char *proxy_name,
   }
 
   /* get timeout */
-  if(data->set.timeout && data->set.connecttimeout) {
+  timeout = Curl_timeleft(conn, NULL, TRUE);
-    if(data->set.timeout < data->set.connecttimeout)
-      timeout = data->set.timeout;
-    else
-      timeout = data->set.connecttimeout;
-  }
-  else if(data->set.timeout)
-    timeout = data->set.timeout;
-  else if(data->set.connecttimeout)
-    timeout = data->set.connecttimeout;
-  else
-    timeout = DEFAULT_CONNECT_TIMEOUT;
 
   Curl_nonblock(sock, TRUE);
 

lib/ssluse.c

@@ -1448,40 +1448,17 @@ ossl_connect_step2(struct connectdata *conn,
 {
   struct SessionHandle *data = conn->data;
   int err;
-  long has_passed;
   struct ssl_connect_data *connssl = &conn->ssl[sockindex];
 
   DEBUGASSERT(ssl_connect_2 == connssl->connecting_state
              || ssl_connect_2_reading == connssl->connecting_state
              || ssl_connect_2_writing == connssl->connecting_state);
 
-  /* Find out if any timeout is set. If not, use 300 seconds.
+  /* Find out how much more time we're allowed */
-     Otherwise, figure out the most strict timeout of the two possible one
+  *timeout_ms = Curl_timeleft(conn, NULL, TRUE);
-     and then how much time that has elapsed to know how much time we
-     allow for the connect call */
-  if(data->set.timeout && data->set.connecttimeout) {
-    /* get the most strict timeout of the ones converted to milliseconds */
-    if(data->set.timeout<data->set.connecttimeout)
-      *timeout_ms = data->set.timeout;
-    else
-      *timeout_ms = data->set.connecttimeout;
-  }
-  else if(data->set.timeout)
-    *timeout_ms = data->set.timeout;
-  else if(data->set.connecttimeout)
-    *timeout_ms = data->set.connecttimeout;
-  else
-    /* no particular time-out has been set */
-    *timeout_ms = DEFAULT_CONNECT_TIMEOUT;
-
-  /* Evaluate in milliseconds how much time that has passed */
-  has_passed = Curl_tvdiff(Curl_tvnow(), data->progress.t_startsingle);
-
-  /* subtract the passed time */
-  *timeout_ms -= has_passed;
 
   if(*timeout_ms < 0) {
-    /* a precaution, no need to continue if time already is up */
+    /* no need to continue if time already is up */
     failf(data, "SSL connection timeout");
     return CURLE_OPERATION_TIMEDOUT;
   }

lib/tftp.c

@@ -114,7 +114,7 @@ typedef enum {
   TFTP_ERR_ILLEGAL,
   TFTP_ERR_UNKNOWNID,
   TFTP_ERR_EXISTS,
-  TFTP_ERR_NOSUCHUSER,	/* This will never be triggered by this code */
+  TFTP_ERR_NOSUCHUSER,  /* This will never be triggered by this code */
 
   /* The remaining error codes are internal to curl */
   TFTP_ERR_NONE = -100,
@@ -194,12 +194,14 @@ static void tftp_set_timeouts(tftp_state_data_t *state)
 
   struct SessionHandle *data = state->conn->data;
   time_t maxtime, timeout;
+  long timeout_ms;
 
   time(&state->start_time);
+
   if(state->state == TFTP_STATE_START) {
     /* Compute drop-dead time */
-    maxtime = (time_t)(data->set.connecttimeout/1000L?
+    timeout_ms = Curl_timeleft(state->conn, NULL, TRUE);
-                       data->set.connecttimeout/1000L:30);
+    maxtime = (time_t)(timeout_ms + 500) / 1000;
     state->max_time = state->start_time+maxtime;
 
     /* Set per-block timeout to total */
@@ -219,10 +221,13 @@ static void tftp_set_timeouts(tftp_state_data_t *state)
 
   }
   else {
-
     /* Compute drop-dead time */
-    maxtime = (time_t)(data->set.timeout/1000L?
+    timeout_ms = Curl_timeleft(state->conn, NULL, TRUE);
-                       data->set.timeout/1000L:3600);
+    if(timeout_ms > 0)
+      maxtime = (time_t)(timeout_ms + 500) / 1000;
+    else
+      maxtime = 3600;
+
     state->max_time = state->start_time+maxtime;
 
     /* Set per-block timeout to 10% of total */