[svn] Minor cleanup of iread/iwrite.

Published in <sxsk7raj1t2.fsf@florida.arsdigita.de>.

src/ChangeLog

@@ -1,3 +1,19 @@
+2002-04-14  Hrvoje Niksic  <hniksic@arsdigita.com>
+
+	* http.c (gethttp): Ditto.
+
+	* retr.c (retrieve_url): Initialize variables to appease the
+	compiler.
+
+	* gen_sslfunc.c (ssl_iread): Don't handle EINTR when calling
+	select_fd.
+	(ssl_iwrite): Ditto.
+
+	* connect.c (select_fd): Rewrite to handle EINTR.  Set errno to
+	ETIMEDOUT in case of timeout.
+	(iread): No need to handle EINTR when calling select_fd.
+	(iwrite): Ditto.
+
 2002-04-14  Hrvoje Niksic  <hniksic@arsdigita.com>
 
 	* retr.c (retrieve_url): Make sure that POST is not honored for

src/connect.c

@@ -320,27 +320,37 @@ bindport (unsigned short *port, int family)
 }
 
 #ifdef HAVE_SELECT
-/* Wait for file descriptor FD to be readable, MAXTIME being the
+/* Wait for file descriptor FD to be available, timing out after
-   timeout in seconds.  If WRITEP is non-zero, checks for FD being
+   MAXTIME seconds.  "Available" means readable if writep is 0,
-   writable instead.
+   writeable otherwise.
+
+   Returns 1 if FD is available, 0 for timeout and -1 for error.  */
 
-   Returns 1 if FD is accessible, 0 for timeout and -1 for error in
-   select().  */
 int
 select_fd (int fd, int maxtime, int writep)
 {
-  fd_set fds, exceptfds;
+  fd_set fds;
-  struct timeval timeout;
+  fd_set *rd = NULL, *wrt = NULL;
+  struct timeval tmout;
+  int result;
 
   FD_ZERO (&fds);
   FD_SET (fd, &fds);
-  FD_ZERO (&exceptfds);
+  *(writep ? &wrt : &rd) = &fds;
-  FD_SET (fd, &exceptfds);
+
-  timeout.tv_sec = maxtime;
+  tmout.tv_sec = maxtime;
-  timeout.tv_usec = 0;
+  tmout.tv_usec = 0;
-  /* HPUX reportedly warns here.  What is the correct incantation?  */
+
-  return select (fd + 1, writep ? NULL : &fds, writep ? &fds : NULL,
+  do
-		 &exceptfds, &timeout);
+    result = select (fd + 1, rd, wrt, NULL, &tmout);
+  while (result < 0 && errno == EINTR);
+
+  /* When we've timed out, set errno to ETIMEDOUT for the convenience
+     of the caller. */
+  if (result == 0)
+    errno = ETIMEDOUT;
+
+  return result;
 }
 #endif /* HAVE_SELECT */
 
@@ -411,43 +421,29 @@ conaddr (int fd, ip_address *ip)
    and uses select() to timeout the stale connections (a connection is
    stale if more than OPT.TIMEOUT time is spent in select() or
    read()).  */
+
 int
 iread (int fd, char *buf, int len)
 {
   int res;
 
-  do
-    {
 #ifdef HAVE_SELECT
   if (opt.timeout)
-	{
+    if (select_fd (fd, opt.timeout, 0) <= 0)
-	  do
-	    {
-	      res = select_fd (fd, opt.timeout, 0);
-	    }
-	  while (res == -1 && errno == EINTR);
-	  if (res <= 0)
-	    {
-	      /* Set errno to ETIMEDOUT on timeout.  */
-	      if (res == 0)
-		/* #### Potentially evil!  */
-		errno = ETIMEDOUT;
       return -1;
-	    }
-	}
 #endif
+  do
     res = READ (fd, buf, len);
-    }
   while (res == -1 && errno == EINTR);
 
   return res;
 }
 
 /* Write LEN bytes from BUF to FD.  This is similar to iread(), but
-   doesn't bother with select().  Unlike iread(), it makes sure that
+   unlike iread(), it makes sure that all of BUF is actually written
-   all of BUF is actually written to FD, so callers needn't bother
+   to FD, so callers needn't bother with checking that the return
-   with checking that the return value equals to LEN.  Instead, you
+   value equals to LEN.  Instead, you should simply check for -1.  */
-   should simply check for -1.  */
+
 int
 iwrite (int fd, char *buf, int len)
 {
@@ -459,27 +455,13 @@ iwrite (int fd, char *buf, int len)
      innermost loop deals with the same during select().  */
   while (len > 0)
     {
-      do
-	{
 #ifdef HAVE_SELECT
       if (opt.timeout)
-	    {
+	if (select_fd (fd, opt.timeout, 1) <= 0)
-	      do
-		{
-		  res = select_fd (fd, opt.timeout, 1);
-		}
-	      while (res == -1 && errno == EINTR);
-	      if (res <= 0)
-		{
-		  /* Set errno to ETIMEDOUT on timeout.  */
-		  if (res == 0)
-		    errno = ETIMEDOUT;
 	  return -1;
-		}
-	    }
 #endif
+      do
 	res = WRITE (fd, buf, len);
-	}
       while (res == -1 && errno == EINTR);
       if (res <= 0)
 	break;

src/gen_sslfunc.c

@@ -207,56 +207,33 @@ ssl_printerrors (void)
   return ocerr;
 }
 
-/* SSL version of iread. Only exchanged read for SSL_read 
+/* SSL version of iread.  Only exchanged read for SSL_read Read at
-   Read at most LEN bytes from FD, storing them to BUF.  This is
+   most LEN bytes from FD, storing them to BUF. */
-   virtually the same as read(), but takes care of EINTR braindamage
+
-   and uses select() to timeout the stale connections (a connection is
-   stale if more than OPT.TIMEOUT time is spent in select() or
-   read()).  */
 int
 ssl_iread (SSL *con, char *buf, int len)
 {
-  int res;
+  int res, fd;
-  int fd;
   BIO_get_fd (con->rbio, &fd);
-  do
-    {
 #ifdef HAVE_SELECT
   if (opt.timeout && !SSL_pending (con))
-	{
+    if (select_fd (fd, opt.timeout, 0) <= 0)
-	  do
-	    {
-	      res = select_fd (fd, opt.timeout, 0);
-	    }
-	  while (res == -1 && errno == EINTR);
-	  if (res <= 0)
-	    {
-	      /* Set errno to ETIMEDOUT on timeout.  */
-	      if (res == 0)
-		/* #### Potentially evil!  */
-		errno = ETIMEDOUT;
       return -1;
-	    }
-	}
 #endif
+  do
     res = SSL_read (con, buf, len);
-    }
   while (res == -1 && errno == EINTR);
 
   return res;
 }
 
-/* SSL version of iwrite. Only exchanged write for SSL_write 
+/* SSL version of iwrite.  Only exchanged write for SSL_write Write
-   Write LEN bytes from BUF to FD.  This is similar to iread(), but
+   LEN bytes from BUF to FD.  */
-   doesn't bother with select().  Unlike iread(), it makes sure that
+
-   all of BUF is actually written to FD, so callers needn't bother
-   with checking that the return value equals to LEN.  Instead, you
-   should simply check for -1.  */
 int
 ssl_iwrite (SSL *con, char *buf, int len)
 {
-  int res = 0;
+  int res = 0, fd;
-  int fd;
   BIO_get_fd (con->rbio, &fd);
   /* `write' may write less than LEN bytes, thus the outward loop
      keeps trying it until all was written, or an error occurred.  The
@@ -264,28 +241,13 @@ ssl_iwrite (SSL *con, char *buf, int len)
      innermost loop deals with the same during select().  */
   while (len > 0)
     {
-      do
-	{
 #ifdef HAVE_SELECT
       if (opt.timeout)
-	    {
+	if (select_fd (fd, opt.timeout, 1) <= 0)
-	      do
-		{
-		  res = select_fd (fd, opt.timeout, 1);
-		}
-	      while (res == -1 && errno == EINTR);
-	      if (res <= 0)
-		{
-		  /* Set errno to ETIMEDOUT on timeout.  */
-		  if (res == 0)
-		    /* #### Potentially evil!  */
-		    errno = ETIMEDOUT;
 	  return -1;
-		}
-	    }
 #endif
+      do
 	res = SSL_write (con, buf, len);
-	}
       while (res == -1 && errno == EINTR);
       if (res <= 0)
 	break;

src/http.c

@@ -181,7 +181,7 @@ static int
 post_file (int sock, void *ssl, const char *file_name, long promised_size)
 {
   static char chunk[8192];
-  int written = 0;
+  long written = 0;
   int write_error;
   FILE *fp;
 
@@ -196,7 +196,7 @@ post_file (int sock, void *ssl, const char *file_name, long promised_size)
     goto pad;
   while (written < promised_size)
     {
-      long towrite;
+      int towrite;
       int length = fread (chunk, 1, sizeof (chunk), fp);
       if (length == 0)
 	break;
@@ -215,18 +215,19 @@ post_file (int sock, void *ssl, const char *file_name, long promised_size)
       written += towrite;
     }
   fclose (fp);
+
  pad:
   if (written < promised_size)
     {
-      DEBUGP (("padding ... "));
       /* This highly unlikely case can happen only if the file has
-	 shrunk while we weren't looking.  To uphold the promise, pad
+	 shrunk under us.  To uphold the promise that exactly
-	 the remaining data with zeros.  #### Should we abort
+	 promised_size bytes would be delivered, pad the remaining
-	 instead?  */
+	 data with zeros.  #### Should we abort instead?  */
+      DEBUGP (("padding %ld bytes ... ", promised_size - written));
       memset (chunk, '\0', sizeof (chunk));
       while (written < promised_size)
 	{
-	  long towrite = WMIN (promised_size - written, sizeof (chunk));
+	  int towrite = WMIN (promised_size - written, sizeof (chunk));
 #ifdef HAVE_SSL
 	  if (ssl)
 	    write_error = ssl_iwrite (ssl, chunk, towrite);
@@ -644,7 +645,7 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy)
 
   /* Headers sent when using POST. */
   char *post_content_type, *post_content_length;
-  long post_data_size;
+  long post_data_size = 0;
 
 #ifdef HAVE_SSL
   /* initialize ssl_ctx on first run */

src/retr.c

@@ -318,8 +318,8 @@ retrieve_url (const char *origurl, char **file, char **newloc,
   int redirection_count = 0;
 
   int post_data_suspended = 0;
-  char *saved_post_data;
+  char *saved_post_data = NULL;
-  char *saved_post_file_name;
+  char *saved_post_file_name = NULL;
 
   /* If dt is NULL, just ignore it.  */
   if (!dt)