wget: reorder fread and poll: poll only if fread returns EAGAIN. Closes 5426

function                                             old     new   delta
retrieve_file_data                                   451     427     -24

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

diff --git a/networking/wget.c b/networking/wget.c
index 3416636..4eafebe 100644 (file)
--- a/networking/wget.c
+++ b/networking/wget.c
@@ -444,13 +444,10 @@ static void NOINLINE retrieve_file_data(FILE *dfp)
 {
 #if ENABLE_FEATURE_WGET_STATUSBAR || ENABLE_FEATURE_WGET_TIMEOUT
 # if ENABLE_FEATURE_WGET_TIMEOUT
-       unsigned second_cnt;
+       unsigned second_cnt = G.timeout_seconds;
 # endif
        struct pollfd polldata;
 
-# if ENABLE_FEATURE_WGET_TIMEOUT
-       second_cnt = G.timeout_seconds;
-# endif
        polldata.fd = fileno(dfp);
        polldata.events = POLLIN | POLLPRI;
 #endif
@@ -468,7 +465,7 @@ static void NOINLINE retrieve_file_data(FILE *dfp)
                 * which messes up progress bar and/or timeout logic.
                 * Because of nonblocking I/O, we need to dance
                 * very carefully around EAGAIN. See explanation at
-                * clearerr() call.
+                * clearerr() calls.
                 */
                ndelay_on(polldata.fd);
 #endif
@@ -476,34 +473,7 @@ static void NOINLINE retrieve_file_data(FILE *dfp)
                        int n;
                        unsigned rdsz;
 
-                       rdsz = sizeof(G.wget_buf);
-                       if (G.got_clen) {
-                               if (G.content_len < (off_t)sizeof(G.wget_buf)) {
-                                       if ((int)G.content_len <= 0)
-                                               break;
-                                       rdsz = (unsigned)G.content_len;
-                               }
-                       }
-
 #if ENABLE_FEATURE_WGET_STATUSBAR || ENABLE_FEATURE_WGET_TIMEOUT
-                       if (safe_poll(&polldata, 1, 1000) == 0) {
-# if ENABLE_FEATURE_WGET_TIMEOUT
-                               if (second_cnt != 0 && --second_cnt == 0) {
-                                       progress_meter(PROGRESS_END);
-                                       bb_error_msg_and_die("download timed out");
-                               }
-# endif
-                               /* Needed for "stalled" indicator */
-                               progress_meter(PROGRESS_BUMP);
-                               /*
-                                * We used to loop back to poll here,
-                                * but in chunked case, we can be here after
-                                * fgets and it could buffer some data in dfp...
-                                * which poll knows nothing about!
-                                * Therefore let's try fread'ing anyway.
-                                */
-                       }
-
                        /* fread internally uses read loop, which in our case
                         * is usually exited when we get EAGAIN.
                         * In this case, libc sets error marker on the stream.
@@ -513,38 +483,71 @@ static void NOINLINE retrieve_file_data(FILE *dfp)
                         * into if (n <= 0) ...
                         */
                        clearerr(dfp);
-                       errno = 0;
 #endif
+                       errno = 0;
+                       rdsz = sizeof(G.wget_buf);
+                       if (G.got_clen) {
+                               if (G.content_len < (off_t)sizeof(G.wget_buf)) {
+                                       if ((int)G.content_len <= 0)
+                                               break;
+                                       rdsz = (unsigned)G.content_len;
+                               }
+                       }
                        n = fread(G.wget_buf, 1, rdsz, dfp);
-                       /* man fread:
+
+                       if (n > 0) {
+                               xwrite(G.output_fd, G.wget_buf, n);
+#if ENABLE_FEATURE_WGET_STATUSBAR
+                               G.transferred += n;
+#endif
+                               if (G.got_clen) {
+                                       G.content_len -= n;
+                                       if (G.content_len == 0)
+                                               break;
+                               }
+#if ENABLE_FEATURE_WGET_TIMEOUT
+                               second_cnt = G.timeout_seconds;
+#endif
+                               continue;
+                       }
+
+                       /* n <= 0.
+                        * man fread:
                         * If error occurs, or EOF is reached, the return value
                         * is a short item count (or zero).
                         * fread does not distinguish between EOF and error.
                         */
-                       if (n <= 0) {
-#if ENABLE_FEATURE_WGET_STATUSBAR || ENABLE_FEATURE_WGET_TIMEOUT
-                               if (errno == EAGAIN) /* poll lied, there is no data? */
-                                       continue; /* yes */
-#endif
-                               if (ferror(dfp))
+                       if (errno != EAGAIN) {
+                               if (ferror(dfp)) {
+                                       progress_meter(PROGRESS_END);
                                        bb_perror_msg_and_die(bb_msg_read_error);
+                               }
                                break; /* EOF, not error */
                        }
 
-                       xwrite(G.output_fd, G.wget_buf, n);
-#if ENABLE_FEATURE_WGET_TIMEOUT
-                       second_cnt = G.timeout_seconds;
-#endif
-#if ENABLE_FEATURE_WGET_STATUSBAR
-                       G.transferred += n;
+#if ENABLE_FEATURE_WGET_STATUSBAR || ENABLE_FEATURE_WGET_TIMEOUT
+                       /* It was EAGAIN. There is no data. Wait up to one second
+                        * then abort if timed out, or update the bar and try reading again.
+                        */
+                       if (safe_poll(&polldata, 1, 1000) == 0) {
+# if ENABLE_FEATURE_WGET_TIMEOUT
+                               if (second_cnt != 0 && --second_cnt == 0) {
+                                       progress_meter(PROGRESS_END);
+                                       bb_error_msg_and_die("download timed out");
+                               }
+# endif
+                               /* We used to loop back to poll here,
+                                * but there is no great harm in letting fread
+                                * to try reading anyway.
+                                */
+                       }
+                       /* Need to do it _every_ second for "stalled" indicator
+                        * to be shown properly.
+                        */
                        progress_meter(PROGRESS_BUMP);
 #endif
-                       if (G.got_clen) {
-                               G.content_len -= n;
-                               if (G.content_len == 0)
-                                       break;
-                       }
-               }
+               } /* while (reading data) */
+
 #if ENABLE_FEATURE_WGET_STATUSBAR || ENABLE_FEATURE_WGET_TIMEOUT
                clearerr(dfp);
                ndelay_off(polldata.fd); /* else fgets can get very unhappy */
@@ -560,6 +563,13 @@ static void NOINLINE retrieve_file_data(FILE *dfp)
                if (G.content_len == 0)
                        break; /* all done! */
                G.got_clen = 1;
+               /*
+                * Note that fgets may result in some data being buffered in dfp.
+                * We loop back to fread, which will retrieve this data.
+                * Also note that code has to be arranged so that fread
+                * is done _before_ one-second poll wait - poll doesn't know
+                * about stdio buffering and can result in spurious one second waits!
+                */
        }
 
        /* If -c failed, we restart from the beginning,