[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] need some advice for 0 timeout code
From: |
Tim Ruehsen |
Subject: |
Re: [Bug-wget] need some advice for 0 timeout code |
Date: |
Tue, 15 May 2012 09:40:21 +0200 |
User-agent: |
KMail/1.13.7 (Linux/3.2.0-2-amd64; KDE/4.7.4; x86_64; ; ) |
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