[Bug 377] [PATCH]: wget: support for -T; rework alarm() -> poll() in wget.c

Bradley M. Kuhn bkuhn at ebb.org
Sat Aug 7 21:48:31 UTC 2010


This is my first time patching BusyBox, so if there is something wrong
that I've done, please let me know and I'm happy to rewrite the patch or
submit it more correctly according to BusyBox patch submission policies.
(BTW, I couldn't find more instructions on procedure other than:
http://busybox.net/fix.html   Did I miss something on the website?)

In a thread discussing support for the -T option in wget, Denys wrote on
Wed, 19 Nov 2008 17:58:19 -0800:
>> To be honest, whole dance with alarm() in wget needs rewriting,
>> poll(..., seconds*1000) + read() would allow signal-less handling of
>> progressmeter and/or timeout.

I believe I have implemented that now, as Denys described above, in the
attached patch below.  I have also added the -T support for wget, which
was the original impetus for Denys' Nov 2008 comment. The full thread in
question can be found here:
   http://lists.busybox.net/pipermail/busybox/2008-August/066797.html

I note that in that thread that Bernhard Reutner-Fischer said:
> I don't think a config option for the timeout makes sense.

I admit I've ignored that advice because it seemed right to me to make
it a feature.  But, if you'd rather I rewrite the patch to make it not a
feature setting, and because I'd already started working on it that way
before I found the original thread.  I'm of course willing to rewrite
the patch so it doesn't add the -T support as a new feature.

This patch does increase the size of BusyBox by 223 bytes in an
allyesconfig:

function                                             old     new   delta
retrieve_file_data                                   476     651    +175
packed_usage                                       28815   28880     +65
bbconfig_config                                    21196   21226     +30
static.wget_longopts                                 145     155     +10
wget_main                                           2516    2525      +9
progress_meter                                       259     193     -66
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 5/1 up/down: 289/-66)           Total: 223 bytes

However, if you turn off FEATURE_WGET_TIMEOUT, the alarm() -> poll()
rewrite (which Denys requested) adds only 93 bytes:

function                                             old     new   delta
retrieve_file_data                                   476     584    +108
bbconfig_config                                    21196   21237     +41
static.wget_longopts                                 145     155     +10
progress_meter                                       259     193     -66
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 3/1 up/down: 159/-66)            Total: 93 bytes


BTW, I didn't write any tests for this because the only way reliable I
found to test the -T option was to use tc over localhost, ala:
   /sbin/tc qdisc add dev lo root handle 1:0 netem delay 30sec

I wasn't sure if you wanted the test suite to depend on tc in that way.
But if that's ok, I am willing to write up a test file, too.

Please let me know if I've submitted this patch wrong as well.

I attest that I am the sole copyright holder on the attached patch, and
I license it under GPLv2-or-later, but I do understand that BusyBox as a
whole is currently GPLv2-only.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: bkuhn-wget-FEATURE_WGET_TIMEOUT-support-T.patch
Type: text/x-diff
Size: 8039 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20100807/ee6bd4c8/attachment.bin>
-------------- next part --------------
-- 
Bradley M. Kuhn, President, Software Freedom Conservancy


More information about the busybox mailing list