[Top][All Lists]

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

Re: [Bug-wget] Disable assertions by default

From: Darshit Shah
Subject: Re: [Bug-wget] Disable assertions by default
Date: Fri, 21 Nov 2014 15:46:36 +0530
User-agent: Mutt/1.5.23 (2014-03-12)

On 11/21, Tim Rühsen wrote:
On Friday 21 November 2014 13:19:18 Darshit Shah wrote:
On 11/20, Ángel González wrote:
>On 20/11/14 15:29, Darshit Shah wrote:
>>--- a/src/progress.c
>>+++ b/src/progress.c
>>@@ -992,6 +992,7 @@ create_image (struct bar_progress *bp, double
>>dl_total_time, bool done)>>
>>      {
>>        int percentage = 100.0 * size / bp->total_length;
>>        assert (percentage<= 100);
>>+      assert (false);
>>        if (percentage<  100)
>>          sprintf (p, "%3d%%", percentage);
>>-- 2.1.3
>Surely you didn't want to include this :)

Shit! No, that was meant to be for my own debugging purposes. I was trying
to see if I could induce an assertion failure. Good thing the patch goes
through review first.

I've fixed this in the attached patch.

Just a comment.
In case the assertions are disabled, there still should be a check and a
WARNING message. It should inform the user that something weird happened and
the mailing list should be informed. Wget should try to continue if possible.
We just had the report that an assertion occurred after hours (and many GB) of
downloading and Wget just stopped... It was just one file out of thousands
that would have been skipped...

I agree. We should add more logging and a warning message for when a file cannot be downloaded. And for such recursive cases, the same information should be displayed at the end of the operation too. This is currently missing.

IMHO. we should have something like this ASAP. And having this, we also might
get rid of assertions completely. That could make this patch obsolete.

I think we should not get rid of assertions. Some of the assertions, like the one at init.c:819 or progress.c:1180 are not for handling run-time errors but for intimating future developers about a logical error immediately. Such checks need to remain in the development code, but is worthless in production code.

Which is why I believe that assertions should remain. Just not all the ones we currently have.

Let's get this patch through first and others to handle the old assertions can flow in over the next week.

Thanking You,
Darshit Shah

Attachment: pgpKtljhd8qXF.pgp
Description: PGP signature

reply via email to

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