Re: [Bug-wget] Disable assertions by default

[Darshit Shah]

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

pgpKtljhd8qXF.pgp
Description: PGP signature