Re: [Bug-wget] need some advice for 0 timeout code

[Tim Ruehsen]

Hi Giuseppe,

Am Monday 14 May 2012 schrieb Giuseppe Scrivano:
> Hi Tim,
> 
> Tim Ruehsen <address@hidden> writes:
> > There are three obvious ways to fix the issue once and for all:
> > a)
> > when given the timeout value 0, use INFINITY and special handle this
> > value in select_fd() to call select with timeout NULL.
> > - INFINITY is defined in math.h which needs to be included in connect.c
> > and init.c.
> > - cmd_time() maybe needs a clone to set a 0 value to INFINITY.
> > - checks like if (timeout) have to be changed into if
> > (timeout!=INFINITY).
> > 
> > b)
> > when given the timeout value 0, use a very high timeout value like 100
> > years (maybe larger to handle future extrasolar communication ;-).
> > - since the code for timeout!=0 is very well tested since it is the
> > normal case, we won't *need* any further changes.
> > - we could optionally get rid of the if (timeout) ... extra code in
> > connect.c and gnutls (but get some extra calls to select()).
> > 
> > c) just fix gnutls.c.
> > - similar to a), but limited to gnutls.c and connect.c.
> > - gnutls seem to need NONBLOCKING sockets, so we would call select() /
> > select_fd() anyways. Again unneccessary extra code to maintain...
> 
> I would rather explictly handle the timeout == 0 case.  It has the
> advantage to "describe" what the program is doing and avoid some magic
> numbers (like 100 years or 10 centuries).
> 
> I find it slightly clearer to read code like this:
> 
>   if (timeout)
>     result = select (maxfd + 1, &rds, NULL, NULL, &tm);
>   else /* No timeout was specified.  */
>     result = select (maxfd + 1, &rds, NULL, NULL, NULL);
> 
> than:
> 
>   if (timeout)
>     tm.tv_sec = 100 * 365 * 24 * 60 * 60; /* Around 100 years.  */
> 
>   result = select (maxfd + 1, &rds, NULL, NULL, &tm);
> 
> That is just my personal taste though on a particular point, please let
> me know if you disagree on it.

I would agree with you and already tried this solution at the beginning of my 
investigation.

But it does not work this way, since timeout==0 is already used as 'zero-
timeout' aka 'polling'. It would involve several critical changes to well-
tested code - which I would like to avoid. Second, we would also need a 
special value for 'polling', e.g. -1 (which is also already used somewhere as 
timeout value).

Maybe something went wrong in the design phase of wget.
E.g. why are sockets BLOCKING - they should all be NONBLOCKING by default. 
Instead of using select() with 0 timeout we would directly call the 
plain/openssl/gnutls 'read' function to see if there is data or if the socket 
has been closed. And than we could straightforward use timeout==0 for an 
INIFINTE select.

I do not have enough overview at the moment to suggest such a deep change.
Maybe it is a task for wget 2.0 ?

> Thanks,
> Giuseppe

Regards, Tim