[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Bug-wget] broken progressbar in 1.16

From: Darshit Shah
Subject: Re: [Bug-wget] broken progressbar in 1.16
Date: Sun, 16 Nov 2014 11:53:30 +0530
User-agent: Mutt/1.5.23 (2014-03-12)

On 11/15, Darshit Shah wrote:
On 11/07, Darshit Shah wrote:
The last patch breaks the download but is only available on the git
repository and will not be included in any release.

I will be pushing a patch soon which will remove all assert statements from
production releases so that the downloads continue with a minor visual

Because real life is catching up, I've very little time till the end of
this year. But I'm going to try and debug this issue and fix it asap.

And eventually I found time to look into this with a fresh mind.
It turns out that I have been looking at the code in a wrong fashion, and hence the assumption I wished to validate with the assert was wrong too. A simple off-by-one error that made my life living hell.

I've fixed the issue with the following patch. Please take a look at it.

I really hope there's no more issues remaining with the progress bar.

Since no one objected, pushed!

On 06-Nov-2014 7:02 pm, "Noël Köthe" <address@hidden> wrote:


Am Montag, den 03.11.2014, 10:12 +0530 schrieb Darshit Shah:

>e-flightgear-201411   0%[                      ]   1,05M   286KB/s
eta 1h 41m
>wget: progress.c:1161: create_image: Проверочное утверждение
«count_cols (bp->buffer) <= bp->width» не выполнено.
>zsh: abort      wget -c
>(it's [Assertion "count_cols (bp->buffer) <= bp->width" failed] I

This bug keeps getting more interesting and surprising. While the
progress bar
draws correctly for the original file you mentioned, the assertion
fails with
the new ISO (live-flightgear). The funny thing is, this bug is now
firing based
on the filename of the file being downloaded, which is not something I
since the filename section of the progress bar is isolated from the
rest and
should not create such an effect.

I'm going to spend some more time debugging this issue.

I applied the progress bar patch on 1.16 and run into the same problem.

Connecting to fly.osdn.org.ua (fly.osdn.org.ua)||:80...
HTTP request sent, awaiting response... 206 Partial Content
Length: 1599602688 (1.5G), 1590685248 (1.5G) remaining
Saving to: 'live-flightgear-20141101-x86_64.iso'

ar-20141101-x86_64.   0%[                      ]  12.47M  1.41MB/s
     wget: progress.c:1161: create_image: Assertion `count_cols
(bp->buffer) <= bp->width' failed.

Looks like shipping wget with the broken progress bar because the patch
could stop downloading.
Maybe the default could be the old but stable progress bar and when the
new feature works it could be changed to the new default?




I added the assertion there to ensure that the basic assumption about the
progress bar is always held. When I added the assert statement, the only
time it was expected to fail was when wget is executed using a non English

However, through my own experience and via other reports on this mailing
list, I have to realize that the assert fails even on English locale and
when the progress bar displays correctly. The fact that the progress bar
manages to print on a single line even when the assertion that
`no_of_cols(progress_bar) <= columns_in_line` fails means that there is a
subtle bug hidden deep in the implementation which is beyond the changes I
made. It also implies that we do not currently know how this code works
since it performs contrary to our expectation.

This is definitely a high priority issue that needs more investigation and
a fix as soon as possible.
--- end quoted text ---

Thanking You,
Darshit Shah

From 94805ad55ae53039717a7f705fe41f0cbe0d8ebc Mon Sep 17 00:00:00 2001
From: Darshit Shah <address@hidden>
Date: Sat, 15 Nov 2014 00:13:13 +0530
Subject: [PATCH 1/2] Fix progress bar assertion

src/ChangeLog  |  7 ++++++-
src/progress.c | 10 +++++++++-
2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 32eebda..d085953 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2014-11-15  Darshit Shah  <address@hidden>
+       * progress.c (create_image): Fix assertion that checks progress bar 
+       Should fix bug #43593
2014-11-12  Tim Ruehsen  <address@hidden>

        * openssl.c (ssl_init): Fix error handling for CRL loading
@@ -7,7 +12,7 @@
        * html-parse.c (map_html_tags): Fix range check

2014-11-11  Tim Ruehsen  <address@hidden>
- +
        * openssl.c (ssl_init): Load CRL file given by --crl-file

2014-11-11  Tim Ruehsen  <address@hidden>
diff --git a/src/progress.c b/src/progress.c
index a0b48e4..d23654d 100644
--- a/src/progress.c
+++ b/src/progress.c
@@ -1158,7 +1158,15 @@ create_image (struct bar_progress *bp, double 
dl_total_time, bool done)
  while (p - bp->buffer - bytes_cols_diff < bp->width)
    *p++ = ' ';
  *p = '\0';
-  assert (count_cols (bp->buffer) <= bp->width);
+  /* 2014-11-14  Darshit Shah  <address@hidden>
+   * Assert that the length of the progress bar is lesser than the size of the
+   * screen with which we are dealing. This assertion *MUST* always be removed
+   * from the release code since we do not want Wget to crash and burn when the
+   * assertion fails. Instead Wget should continue downloading and display a
+   * horrible and irritating progress bar that spams the screen with newlines.
+   */
+  assert (count_cols (bp->buffer) <= bp->width + 1);

/* Print the contents of the buffer as a one-line ASCII "image" so

--- end quoted text ---

Thanking You,
Darshit Shah

Attachment: pgpSvLGQErZBG.pgp
Description: PGP signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]