|
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:
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.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 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.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.
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
pgpKtljhd8qXF.pgp
Description: PGP signature
[Prev in Thread] | Current Thread | [Next in Thread] |