[svn] Better documentation for fd_read_body args.

src/ChangeLog

@@ -1,3 +1,9 @@
+2003-11-30  Hrvoje Niksic  <hniksic@xemacs.org>
+
+	* retr.c (fd_read_body): Sanitize arguments and document them
+	better.  Make sure the timer is created and updated only if
+	necessary.  Updated callers.
+
 2003-11-30  Hrvoje Niksic  <hniksic@xemacs.org>
 
 	* http.c (skip_body): New function.

src/ftp.c

@@ -1023,8 +1023,11 @@ Error in server response, closing control connection.\n"));
     }
 
   /* Get the contents of the document.  */
-  res = fd_read_body (dtsock, fp, len, restval, expected_bytes, 0,
+  res = fd_read_body (dtsock, fp,
-		      &con->dltime);
+		      expected_bytes ? expected_bytes - restval : 0,
+		      0, restval, len, &con->dltime);
+  *len += restval;
+
   tms = time_str (NULL);
   tmrate = retr_rate (*len - restval, con->dltime, 0);
   /* Close data connection socket.  */

src/http.c

@@ -759,7 +759,7 @@ skip_body (int fd, long contlen)
 
   oldverbose = opt.verbose;
   opt.verbose = 0;
-  fd_read_body (fd, NULL, &dummy, 0, contlen, 1, NULL);
+  fd_read_body (fd, NULL, contlen, 1, 0, &dummy, NULL);
   opt.verbose = oldverbose;
 }
 
@@ -1407,7 +1407,8 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy)
     }
   logprintf (LOG_VERBOSE, _("%s request sent, awaiting response... "),
 	     proxy ? "Proxy" : "HTTP");
-  contlen = contrange = -1;
+  contlen = -1;
+  contrange = 0;
   type = NULL;
   statcode = -1;
   *dt &= ~RETROKF;
@@ -1612,31 +1613,30 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy)
 	}
     }
 
-  if (contrange == -1)
+  if (contrange == 0 && hs->restval > 0)
     {
-      /* We did not get a content-range header.  This means that the
+      /* The download starts from the beginning, presumably because
-	 server did not honor our `Range' request.  Normally, this
+	 the server did not honor our `Range' request.  Normally we'd
-	 means we should reset hs->restval and continue normally.  */
+	 just reset hs->restval and start the download from
+	 scratch.  */
 
       /* However, if `-c' is used, we need to be a bit more careful:
 
          1. If `-c' is specified and the file already existed when
-         Wget was started, it would be a bad idea for us to start
+         Wget was started, it would be a bad idea to start downloading
-         downloading it from scratch, effectively truncating it.  I
+         it from scratch, effectively truncating the file.
-         believe this cannot happen unless `-c' was specified.
 
 	 2. If `-c' is used on a file that is already fully
 	 downloaded, we're requesting bytes after the end of file,
-	 which can result in server not honoring `Range'.  If this is
+	 which can result in the server not honoring `Range'.  If this
-	 the case, `Content-Length' will be equal to the length of the
+	 is the case, `Content-Length' will be equal to the length of
-	 file.  */
+	 the file.  */
       if (opt.always_rest)
 	{
 	  /* Check for condition #2. */
-	  if (hs->restval > 0	            /* restart was requested. */
+	  if (contlen != -1              /* we got content-length. */
-	      && contlen != -1              /* we got content-length. */
+	      && hs->restval >= contlen  /* file fully downloaded
-	      && hs->restval >= contlen     /* file fully downloaded
+					    or has shrunk.  */
-					       or has shrunk.  */
 	      )
 	    {
 	      logputs (LOG_VERBOSE, _("\
@@ -1679,16 +1679,7 @@ Refusing to truncate existing file `%s'.\n\n"), *hs->local_file);
       CLOSE_INVALIDATE (sock);
       return RANGEERR;
     }
-
+  hs->contlen = contlen + contrange;
-  if (hs->restval)
-    {
-      if (contlen != -1)
-	contlen += contrange;
-      else
-	contrange = -1;        /* If conent-length was not sent,
-				  content-range will be ignored.  */
-    }
-  hs->contlen = contlen;
 
   if (opt.verbose)
     {
@@ -1700,10 +1691,9 @@ Refusing to truncate existing file `%s'.\n\n"), *hs->local_file);
 	  logputs (LOG_VERBOSE, _("Length: "));
 	  if (contlen != -1)
 	    {
-	      logputs (LOG_VERBOSE, legible (contlen));
+	      logputs (LOG_VERBOSE, legible (contlen + contrange));
-	      if (contrange != -1)
+	      if (contrange)
-		logprintf (LOG_VERBOSE, _(" (%s to go)"),
+		logprintf (LOG_VERBOSE, _(" (%s to go)"), legible (contlen));
-			   legible (contlen - contrange));
 	    }
 	  else
 	    logputs (LOG_VERBOSE,
@@ -1785,10 +1775,10 @@ Refusing to truncate existing file `%s'.\n\n"), *hs->local_file);
   if (opt.save_headers)
     fwrite (head, 1, strlen (head), fp);
 
-  /* Get the contents of the document.  */
+  /* Download the request body.  */
-  hs->res = fd_read_body (sock, fp, &hs->len, hs->restval,
+  hs->res = fd_read_body (sock, fp, contlen != -1 ? contlen : 0, keep_alive,
-			  (contlen != -1 ? contlen : 0),
+			  hs->restval, &hs->len, &hs->dltime);
-			  keep_alive, &hs->dltime);
+  hs->len += contrange;
 
   if (hs->res >= 0)
     CLOSE_FINISH (sock);

src/retr.c

@@ -133,32 +133,35 @@ limit_bandwidth (long bytes, struct wget_timer *timer)
 # define MIN(i, j) ((i) <= (j) ? (i) : (j))
 #endif
 
-/* Reads the contents of file descriptor FD, until it is closed, or a
+/* Read the contents of file descriptor FD until it the connection
-   read error occurs.  The data is read in 8K chunks, and stored to
+   terminates or a read error occurs.  The data is read in portions of
-   stream fp, which should have been open for writing.
+   up to 16K and written to OUT as it arrives.  If opt.verbose is set,
+   the progress is shown.
 
-   The EXPECTED argument is passed to show_progress() unchanged, but
+   TOREAD is the amount of data expected to arrive, normally only used
-   otherwise ignored.
+   by the progress gauge.  However, if EXACT is set, no more than
+   TOREAD octets will be read.
 
-   If opt.verbose is set, the progress is also shown.  RESTVAL
+   STARTPOS is the position from which the download starts, used by
-   (RESTart VALue) is the position from which the download starts,
+   the progress gauge.  The amount of data read gets stored to
-   needed for progress display.
+   *AMOUNT_READ.  The time it took to download the data (in
+   milliseconds) is stored to *ELAPSED.
 
-   The function exits and returns codes of 0, -1 and -2 if the
+   The function exits and returns the amount of data read.  In case of
-   connection was closed, there was a read error, or if it could not
+   error while reading data, -1 is returned.  In case of error while
-   write to the output stream, respectively.  */
+   writing data, -2 is returned.  */
 
 int
-fd_read_body (int fd, FILE *out, long *len, long restval, long expected,
+fd_read_body (int fd, FILE *out, long toread, int exact, long startpos,
-	      int use_expected, double *elapsed)
+	      long *amount_read, double *elapsed)
 {
-  int res = 0;
+  int ret = 0;
 
   static char dlbuf[16384];
   int dlbufsize = sizeof (dlbuf);
 
-  struct wget_timer *timer = wtimer_allocate ();
+  struct wget_timer *timer = NULL;
-  double last_successful_read_tm;
+  double last_successful_read_tm = 0;
 
   /* The progress gauge, set according to the user preferences. */
   void *progress = NULL;
@@ -169,18 +172,25 @@ fd_read_body (int fd, FILE *out, long *len, long restval, long expected,
      data arrives slowly. */
   int progress_interactive = 0;
 
-  *len = restval;
+  *amount_read = 0;
 
   if (opt.verbose)
     {
-      progress = progress_create (restval, expected);
+      progress = progress_create (startpos, toread);
       progress_interactive = progress_interactive_p (progress);
     }
 
   if (opt.limit_rate)
     limit_bandwidth_reset ();
-  wtimer_reset (timer);
+
-  last_successful_read_tm = 0;
+  /* A timer is needed for tracking progress, for throttling, and for
+     tracking elapsed time.  If either of these are requested, start
+     the timer.  */
+  if (progress || opt.limit_rate || elapsed)
+    {
+      timer = wtimer_new ();
+      last_successful_read_tm = 0;
+    }
 
   /* Use a smaller buffer for low requested bandwidths.  For example,
      with --limit-rate=2k, it doesn't make sense to slurp in 16K of
@@ -189,15 +199,13 @@ fd_read_body (int fd, FILE *out, long *len, long restval, long expected,
   if (opt.limit_rate && opt.limit_rate < dlbufsize)
     dlbufsize = opt.limit_rate;
 
-  /* Read from FD while there is available data.
+  /* Read from FD while there is data to read.  Normally toread==0
-
+     means that it is unknown how much data is to arrive.  However, if
-     Normally, if expected is 0, it means that it is not known how
+     EXACT is set, then toread==0 means what it says: that no data
-     much data is expected.  However, if use_expected is specified,
+     should be read.  */
-     then expected being zero means exactly that.  */
+  while (!exact || (*amount_read < toread))
-  while (!use_expected || (*len < expected))
     {
-      int amount_to_read = (use_expected
+      int rdsize = exact ? MIN (toread - *amount_read, dlbufsize) : dlbufsize;
-			    ? MIN (expected - *len, dlbufsize) : dlbufsize);
       double tmout = opt.read_timeout;
       if (progress_interactive)
 	{
@@ -214,61 +222,64 @@ fd_read_body (int fd, FILE *out, long *len, long restval, long expected,
 	      if (tmout < 0)
 		{
 		  /* We've already exceeded the timeout. */
-		  res = -1, errno = ETIMEDOUT;
+		  ret = -1, errno = ETIMEDOUT;
 		  break;
 		}
 	    }
 	}
-      res = fd_read (fd, dlbuf, amount_to_read, tmout);
+      ret = fd_read (fd, dlbuf, rdsize, tmout);
 
-      if (res == 0 || (res < 0 && errno != ETIMEDOUT))
+      if (ret == 0 || (ret < 0 && errno != ETIMEDOUT))
 	break;
-      else if (res < 0)
+      else if (ret < 0)
-	res = 0;		/* timeout */
+	ret = 0;		/* timeout */
 
-      wtimer_update (timer);
+      if (progress || opt.limit_rate)
-      if (res > 0)
 	{
-	  if (out)
+	  wtimer_update (timer);
+	  if (ret > 0)
+	    last_successful_read_tm = wtimer_read (timer);
+	}
+
+      if (ret > 0 && out != NULL)
+	{
+	  fwrite (dlbuf, 1, ret, out);
+	  /* Immediately flush the downloaded data.  This should not
+	     hinder performance: fast downloads will arrive in large
+	     16K chunks (which stdio would write out anyway), and slow
+	     downloads wouldn't be limited by disk speed.  */
+	  fflush (out);
+	  if (ferror (out))
 	    {
-	      fwrite (dlbuf, 1, res, out);
+	      ret = -2;
-	      /* Always flush the contents of the network packet.
+	      goto out;
-		 This should not hinder performance: fast downloads
-		 will be received in 16K chunks (which stdio would
-		 write out anyway), and slow downloads won't be
-		 limited by disk performance.  */
-	      fflush (out);
-	      if (ferror (out))
-		{
-		  res = -2;
-		  goto out;
-		}
 	    }
-	  last_successful_read_tm = wtimer_read (timer);
 	}
 
       if (opt.limit_rate)
-	limit_bandwidth (res, timer);
+	limit_bandwidth (ret, timer);
 
-      *len += res;
+      *amount_read += ret;
       if (progress)
-	progress_update (progress, res, wtimer_read (timer));
+	progress_update (progress, ret, wtimer_read (timer));
 #ifdef WINDOWS
-      if (use_expected && expected > 0)
+      if (toread > 0)
-	ws_percenttitle (100.0 * (double)(*len) / (double)expected);
+	ws_percenttitle (100.0 *
+			 (startpos + *amount_read) / (startpos + toread));
 #endif
     }
-  if (res < -1)
+  if (ret < -1)
-    res = -1;
+    ret = -1;
 
  out:
   if (progress)
     progress_finish (progress, wtimer_read (timer));
   if (elapsed)
     *elapsed = wtimer_read (timer);
-  wtimer_delete (timer);
+  if (timer)
+    wtimer_delete (timer);
 
-  return res;
+  return ret;
 }
 
 /* Read a hunk of data from FD, up until a terminator.  The terminator

src/retr.h

@@ -35,7 +35,7 @@ typedef const char *(*hunk_terminator_t) PARAMS ((const char *, int, int));
 char *fd_read_hunk PARAMS ((int, hunk_terminator_t, int));
 char *fd_read_line PARAMS ((int));
 
-int fd_read_body PARAMS ((int, FILE *, long *, long, long, int, double *));
+int fd_read_body PARAMS ((int, FILE *, long, int, long, long *, double *));
 
 uerr_t retrieve_url PARAMS ((const char *, char **, char **,
 			     const char *, int *));