Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
but even using $@ whitespace in a URL must be correctly escaped or it will not work with busybox wget and with real wget. The reason for this is the point when arguments get expanded or split into words. At this point, a shameless plug feels appropriate. The execline language was precisely made for this kind of script: it spares users the hassle of understanding shell quoting, it does the right thing in every case ; and for such small scripts, launching a shell brings non-negligible overhead, whereas launching an execline script costs practically nothing. #!/command/execlineb -S0 busybox wget -T xxx $@ does exactly what it looks like it should be doing, even if arguments contain special characters, and without any unnecessary system call overhead. http://skarnet.org/software/execline/ for the potentially interested readers. Both of them. -- Laurent ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
On Friday 18 October 2013 23:21, Harald Becker wrote: Hi Tito ! IMO you got the best solution, ... one more solution without the need to modify scripts is to create a wrapper to wget: #!/bin/sh busybox wget -T xxx $@ ... except, I would do an exec here: #!/bin/sh exec busybox wget -T xxx $@ (bike shed painting fest, can't resist to join) Also it's better to expand argumanet properly wrt whitespace - use $@, not $@: exec busybox wget -T xxx $@ ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
On Sunday 20 October 2013 14:57:44 Denys Vlasenko wrote: On Friday 18 October 2013 23:21, Harald Becker wrote: Hi Tito ! IMO you got the best solution, ... one more solution without the need to modify scripts is to create a wrapper to wget: #!/bin/sh busybox wget -T xxx $@ ... except, I would do an exec here: #!/bin/sh exec busybox wget -T xxx $@ (bike shed painting fest, can't resist to join) Also it's better to expand argumanet properly wrt whitespace - use $@, not $@: exec busybox wget -T xxx $@ Hi, but even using $@ whitespace in a URL must be correctly escaped or it will not work with busybox wget and with real wget. Busybox wget with wrapper and $@ and URL escaped: ./wget -c ftp://192.168.1.1/STORE_N_GO/test/Text\ File.txt Connecting to 192.168.1.1 (192.168.1.1:21) Text File.txt100% |**| 2 --:--:-- ETA Busybox wget with wrapper and $@ and URL not escaped: ./wget ftp://192.168.1.1/STORE_N_GO/test/Text File.txt Connecting to 192.168.1.1 (192.168.1.1:21) wget: bad response to RETR: 550 Failed to open file. Ciao, Tito ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
Hi Tito ! #!/bin/sh exec busybox wget -T xxx $@ Also it's better to expand argumanet properly wrt whitespace - use $@, not $@: exec busybox wget -T xxx $@ Denys, you are right, I missed that topic! Using argument quotes is my usual style. but even using $@ whitespace in a URL must be correctly escaped or it will not work with busybox wget and with real wget. The reason for this is the point when arguments get expanded or split into words. Busybox wget with wrapper and $@ and URL escaped: ./wget -c ftp://192.168.1.1/STORE_N_GO/test/Text\ File.txt Here you pass the wrapper two arguments: arg[1] = '-c' arg[2] = 'ftp:.../Text File.txt' Quoting those arguments in the script wrapper avoids those arguments being split into three words: With $@ the wget (program) would receive: arg[1] = '-T' arg[2] = 'xxx' arg[3] = '-c' arg[4] = 'ftp:.../Text' arg[5] = 'File.txt' With $@ the wget program receive (as expected): arg[1] = '-T' arg[2] = 'xxx' arg[3] = '-c' arg[4] = 'ftp:.../Text File.txt' Busybox wget with wrapper and $@ and URL not escaped: ./wget ftp://192.168.1.1/STORE_N_GO/test/Text File.txt The reason here: The URL is getting split into two arguments by the *calling* shell. The wrapper receive: arg[1] = '-c' arg[2] = 'ftp:.../Text' arg[3] = 'File.txt' As the arguments don't contain further characters which require qouting using $@ gives same results as $@. Using $@ avoids unexpected word splitting when calling shell/program passes arguments containing space or other special characters. -- Harald ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
Hi Lauri ! GNU wget does kinda cater to this use: you can specify it in the .wgetrc file, but then that's only portable to GNU wget. How about adding limited support for .wgetrc then (only for timeouts)? Support for .wgetrc is much bloat. What about looking for environment variable WGET_TIMEOUT? Ok it is not compatible with other other wget versions, but BB wget isn't neither. Checking for an env var would be more size. More size than compile time constant, but much less then supporting a .wgetrc file. Having it as a compile-time configure option means no size change, and we don't need the default to change at runtime. ... but you need to think about this timeout before building Busybox and can't change that afterward in runtime environment. So another option to think about before build, you actually don't care during first config. And as soon as you use Busybox wget and know the value, you need to reconfigure/build. IMO it is best to have a sane, all purpose compile time constant, and the possibility to overwrite this constant during runtime with an environment variable. There is nothing wrong to have such a constant as a #define at start of the applet source, but do we pollute the configuration step with this constant? -- Harald ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
On Friday 18 October 2013 11:00:12 Harald Becker wrote: Hi Lauri ! GNU wget does kinda cater to this use: you can specify it in the .wgetrc file, but then that's only portable to GNU wget. How about adding limited support for .wgetrc then (only for timeouts)? Support for .wgetrc is much bloat. What about looking for environment variable WGET_TIMEOUT? Ok it is not compatible with other other wget versions, but BB wget isn't neither. Checking for an env var would be more size. More size than compile time constant, but much less then supporting a .wgetrc file. Having it as a compile-time configure option means no size change, and we don't need the default to change at runtime. ... but you need to think about this timeout before building Busybox and can't change that afterward in runtime environment. So another option to think about before build, you actually don't care during first config. And as soon as you use Busybox wget and know the value, you need to reconfigure/build. IMO it is best to have a sane, all purpose compile time constant, and the possibility to overwrite this constant during runtime with an environment variable. There is nothing wrong to have such a constant as a #define at start of the applet source, but do we pollute the configuration step with this constant? Hi, can the wget scripts not be modified like [ -r /etc/default/wget.conf ] . /etc/default/wget.conf wget -t $WGET_TIMEOUT Ciao, Tito ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
On Fri, Oct 18, 2013, at 12:52, Tito wrote: Hi, can the wget scripts not be modified like [ -r /etc/default/wget.conf ] . /etc/default/wget.conf wget -t $WGET_TIMEOUT Hi Tito, The whole point of this patch was to avoid having to modify scripts, otherwise we'd just have added the lot of -T options. - Lauri -- http://www.fastmail.fm - Faster than the air-speed velocity of an unladen european swallow ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
On Friday 18 October 2013 17:56:57 you wrote: On Fri, Oct 18, 2013, at 12:52, Tito wrote: Hi, can the wget scripts not be modified like [ -r /etc/default/wget.conf ] . /etc/default/wget.conf wget -t $WGET_TIMEOUT Hi Tito, The whole point of this patch was to avoid having to modify scripts, otherwise we'd just have added the lot of -T options. - Lauri Hi, one more solution without the need to modify scripts is to create a wrapper to wget: #!/bin/sh busybox wget -T xxx $@ name it wget and remove the wget link to busybox. Ciao, Tito ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
Hi Tito ! IMO you got the best solution, ... one more solution without the need to modify scripts is to create a wrapper to wget: #!/bin/sh busybox wget -T xxx $@ ... except, I would do an exec here: #!/bin/sh exec busybox wget -T xxx $@ -- Harald ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
On Friday 18 October 2013 23:21:41 Harald Becker wrote: Hi Tito ! IMO you got the best solution, ... one more solution without the need to modify scripts is to create a wrapper to wget: #!/bin/sh busybox wget -T xxx $@ ... except, I would do an exec here: #!/bin/sh exec busybox wget -T xxx $@ -- Harald Hi, Why? works for me. ./wget http://busybox.net/downloads/busybox-1.21.1.tar.bz2 Connecting to busybox.net (140.211.167.224:80) busybox-1.21.1.tar.b 100% |**| 2150k 0:00:00 ETA BTW, this could be even improved to source a global wgetrc file. Ciao, Tito ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
Hi Tito ! exec Busybox wget ... Why? works for me. Works, yes, but needs additional process (and memory resources) for the extra shell (except on some bash versions, tending to optimize the last command). The exec replaces the shell in memory with the wget, this avoids forking and release memory early, otherwise occupied by the shell (until wget exits) -- Harald ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
On Wed, Oct 16, 2013, at 22:23, Harald Becker wrote: GNU wget does kinda cater to this use: you can specify it in the .wgetrc file, but then that's only portable to GNU wget. How about adding limited support for .wgetrc then (only for timeouts)? Just a suggestion ... Support for .wgetrc is much bloat. What about looking for environment variable WGET_TIMEOUT? Ok it is not compatible with other other wget versions, but BB wget isn't neither. Checking for an env var would be more size. Having it as a compile-time configure option means no size change, and we don't need the default to change at runtime. - Lauri -- http://www.fastmail.fm - Or how I learned to stop worrying and love email again ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
On Thursday 17 October 2013 10:48, Lauri Kasanen wrote: On Wed, Oct 16, 2013, at 22:23, Harald Becker wrote: GNU wget does kinda cater to this use: you can specify it in the .wgetrc file, but then that's only portable to GNU wget. How about adding limited support for .wgetrc then (only for timeouts)? Just a suggestion ... Support for .wgetrc is much bloat. What about looking for environment variable WGET_TIMEOUT? Ok it is not compatible with other other wget versions, but BB wget isn't neither. Checking for an env var would be more size. Having it as a compile-time configure option means no size change, and we don't need the default to change at runtime. I don't want to add incompatible changes. The whole point of using same commands on big and on embedded systems is to give user/admin the same environment in both cases. -- vda ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
On Tuesday 15 October 2013 11:05, Lauri Kasanen wrote: On Mon, Oct 14, 2013, at 19:22, Denys Vlasenko wrote: wget: make default timeout configurable Why? Upstream wget uses a fixed default timeout, 900. We'd like to change it in one place instead of adding -T options to a ton of scripts. But this will make your scripts depend on wget being the busybox's wget. This plagued Unix world for some time. Not a good idea. What upstream wget does to address such a case? That's acceptable to us. We call it as busybox wget anyway when it matters, and in other places, it is a mere user experience tweak - it will not break anything if another wget was used. GNU wget does kinda cater to this use: you can specify it in the .wgetrc file, but then that's only portable to GNU wget. How about adding limited support for .wgetrc then (only for timeouts)? ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
On Mon, Oct 14, 2013, at 19:22, Denys Vlasenko wrote: wget: make default timeout configurable Why? Upstream wget uses a fixed default timeout, 900. We'd like to change it in one place instead of adding -T options to a ton of scripts. But this will make your scripts depend on wget being the busybox's wget. This plagued Unix world for some time. Not a good idea. What upstream wget does to address such a case? That's acceptable to us. We call it as busybox wget anyway when it matters, and in other places, it is a mere user experience tweak - it will not break anything if another wget was used. GNU wget does kinda cater to this use: you can specify it in the .wgetrc file, but then that's only portable to GNU wget. - Lauri -- http://www.fastmail.fm - One of many happy users: http://www.fastmail.fm/help/overview_quotes.html ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
On Sunday 13 October 2013 10:12, Lauri Kasanen wrote: On Sat, Oct 12, 2013, at 22:48, Denys Vlasenko wrote: Small housekeeping for wget. wget: make default timeout configurable Why? Upstream wget uses a fixed default timeout, 900. We'd like to change it in one place instead of adding -T options to a ton of scripts. But this will make your scripts depend on wget being the busybox's wget. This plagued Unix world for some time. Not a good idea. What upstream wget does to address such a case? ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
On Sat, Oct 12, 2013, at 22:48, Denys Vlasenko wrote: Small housekeeping for wget. wget: make default timeout configurable Why? Upstream wget uses a fixed default timeout, 900. We'd like to change it in one place instead of adding -T options to a ton of scripts. Doing it via Kconfig is cleaner than changing the source. - Lauri -- http://www.fastmail.fm - The professional email service ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
On Tuesday 08 October 2013 16:14, Lauri Kasanen wrote: Hi, Small housekeeping for wget. wget: make default timeout configurable Why? Upstream wget uses a fixed default timeout, 900. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
Hi,
Small housekeeping for wget.
- Lauri
--
http://www.fastmail.fm - One of many happy users:
http://www.fastmail.fm/help/overview_quotes.html
From 13755e22b0dc9655d5e5d05c2a0d06873a8ef526 Mon Sep 17 00:00:00 2001
From: Lauri Kasanen cur...@operamail.com
Date: Tue, 8 Oct 2013 16:40:20 +0300
Subject: [PATCH 1/2] wget: make default timeout configurable, remove a
duplicate line
Signed-off-by: Lauri Kasanen cur...@operamail.com
---
networking/Config.src | 8
networking/wget.c | 3 +--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/networking/Config.src b/networking/Config.src
index e1ae0c9..0a483ae 100644
--- a/networking/Config.src
+++ b/networking/Config.src
@@ -981,6 +981,14 @@ config FEATURE_WGET_TIMEOUT
connection initialization). When FEATURE_WGET_LONG_OPTIONS is
also enabled, the --timeout option will work in addition to -T.
+config WGET_DEFAULT_TIMEOUT
+ int Default wget timeout
+ default 900
+ range 1 2000
+ depends on FEATURE_WGET_TIMEOUT
+ help
+ The default time, in seconds, to wait before wget gives up.
+
config ZCIP
bool zcip
default y
diff --git a/networking/wget.c b/networking/wget.c
index 5dac2b5..f604828 100644
--- a/networking/wget.c
+++ b/networking/wget.c
@@ -87,7 +87,6 @@ struct globals {
#define G (*ptr_to_globals)
#define INIT_G() do { \
SET_PTR_TO_GLOBALS(xzalloc(sizeof(G))); \
- IF_FEATURE_WGET_TIMEOUT(G.timeout_seconds = 900;) \
} while (0)
@@ -944,7 +943,7 @@ int wget_main(int argc UNUSED_PARAM, char **argv)
INIT_G();
- IF_FEATURE_WGET_TIMEOUT(G.timeout_seconds = 900;)
+ IF_FEATURE_WGET_TIMEOUT(G.timeout_seconds = CONFIG_WGET_DEFAULT_TIMEOUT;)
G.proxy_flag = on; /* use proxies if env vars are set */
G.user_agent = Wget; /* User-Agent header field */
--
1.8.3.1
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
