Re: [OpenWrt-Devel] dnsmasq leasefile fix
On 15.02.2011 11:13, Ferenc Wagner wrote:
Peter Wagner tripo...@gmx.at writes:
- [ -n $leasefile ] [ -e $leasefile ] || touch $leasefile
+ [ -n $leasefile ] ( [ -e $leasefile ] || touch $leasefile )
Looks like this is fixed already by commit 15fba44a (but see point 6 of
http://mywiki.wooledge.org/BashPitfalls and the rest for an interesting
read) so the following is academic, but I typed it before checking...
It's cheeper to use braces in such cases to avoid subshell creation:
[ -n $leasefile ] { [ -e $leasefile ] || touch $leasefile;
}
command grouping is the way to go, right
Btw. is the -e test really necessary? Why not simply
[ -n $leasefile ] touch $leasefile
?
depends on what is more expensive .. the test or the touch, but i doubt the
research is worth the effective result.
ede
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] dnsmasq leasefile fix
Another version of the patch was allready commited - look here
https://dev.openwrt.org/changeset/25540
Am Dienstag, 15. Februar 2011, 20:09:44 schrieb Philip Prindeville:
On 2/15/11 2:13 AM, Ferenc Wagner wrote:
Peter Wagnertripo...@gmx.at writes:
- [ -n $leasefile ] [ -e $leasefile ] || touch $leasefile
+ [ -n $leasefile ] ( [ -e $leasefile ] || touch
$leasefile )
Looks like this is fixed already by commit 15fba44a (but see point 6 of
http://mywiki.wooledge.org/BashPitfalls and the rest for an interesting
read) so the following is academic, but I typed it before checking...
It's cheeper to use braces in such cases to avoid subshell creation:
[ -n $leasefile ] { [ -e $leasefile ] || touch
$leasefile; }
Btw. is the -e test really necessary? Why not simply
[ -n $leasefile ] touch $leasefile
?
Well, to use your own point... why create an extra process to touch a
file that already exists?
And if it does exist, do you necessarily want to modify the timestamp on
it?
I don't like using -e because what happens if the name exists, but it's a
directory or a socket or a block file...
Why not use -f instead?
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] dnsmasq leasefile fix
On 2/15/11 12:18 PM, Felix Fietkau wrote:
On 2011-02-15 8:09 PM, Philip Prindeville wrote:
On 2/15/11 2:13 AM, Ferenc Wagner wrote:
Peter Wagnertripo...@gmx.at writes:
- [ -n $leasefile ] [ -e $leasefile ] || touch $leasefile
+ [ -n $leasefile ] ( [ -e $leasefile ] || touch $leasefile )
Looks like this is fixed already by commit 15fba44a (but see point 6 of
http://mywiki.wooledge.org/BashPitfalls and the rest for an interesting
read) so the following is academic, but I typed it before checking...
It's cheeper to use braces in such cases to avoid subshell creation:
[ -n $leasefile ] { [ -e $leasefile ] || touch
$leasefile; }
Btw. is the -e test really necessary? Why not simply
[ -n $leasefile ] touch $leasefile
?
Well, to use your own point... why create an extra process to touch a file
that already exists?
And if it does exist, do you necessarily want to modify the timestamp on it?
https://dev.openwrt.org/changeset/25540 - committed more than 24 hours
ago, rendering much of this discussion irrelevant ;)
Well, if the discussion offers would-be contributors an inkling of what a
'style guide' would contain, then that's a good thing, right?
I enjoyed the discussion on Bastian's suggestions (perhaps more than poor
Bastian did :-( ) because it gave some insight into what the values of the
maintainers of OpenWRT are.
-Philip
I don't like using -e because what happens if the name exists, but it's a
directory or a socket or a block file...
Why not use -f instead?
If it's a directory or a socket, then touch certainly won't make things
better, so -e is enough, unless you also want to add a conditional rm
-f, as well as some extra checks to make sure it isn't a symlink,
nah, screw that, I don't believe in defensive programming ;)
- Felix
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] dnsmasq leasefile fix
On 2/15/11 1:20 PM, Felix Fietkau wrote:
On 2011-02-15 10:11 PM, Philip Prindeville wrote:
On 2/15/11 12:18 PM, Felix Fietkau wrote:
On 2011-02-15 8:09 PM, Philip Prindeville wrote:
On 2/15/11 2:13 AM, Ferenc Wagner wrote:
Peter Wagnertripo...@gmx.atwrites:
- [ -n $leasefile ][ -e $leasefile ] || touch $leasefile
+ [ -n $leasefile ]( [ -e $leasefile ] || touch $leasefile )
Looks like this is fixed already by commit 15fba44a (but see point 6 of
http://mywiki.wooledge.org/BashPitfalls and the rest for an interesting
read) so the following is academic, but I typed it before checking...
It's cheeper to use braces in such cases to avoid subshell creation:
[ -n $leasefile ]{ [ -e $leasefile ] || touch
$leasefile; }
Btw. is the -e test really necessary? Why not simply
[ -n $leasefile ]touch $leasefile
?
Well, to use your own point... why create an extra process to touch a file
that already exists?
And if it does exist, do you necessarily want to modify the timestamp on it?
https://dev.openwrt.org/changeset/25540 - committed more than 24 hours
ago, rendering much of this discussion irrelevant ;)
Well, if the discussion offers would-be contributors an inkling of
what a 'style guide' would contain, then that's a good thing, right?
I added the ;) for a reason.
Yes, you did.
I enjoyed the discussion on Bastian's suggestions (perhaps more than
poor Bastian did :-( ) because it gave some insight into what the values
of the maintainers of OpenWRT are.
And what are those values?
- Felix
I don't think they'd all be covered in a couple of emails... but it seems to be
an interesting mix of clarity, brevity, and small memory-footprint.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
[OpenWrt-Devel] dnsmasq leasefile fix
hi
when i start dnsmasq i get this error
/etc/init.d/dnsmasq start
touch: : No such file or directory
because i didnt define leasefile in the config file ...
this shouldnt happen as the statement is
[ -n $leasefile ] [ -e $leasefile ] || touch $leasefile
if $leasefile= it will execute touch $leasefile
this patch should fix the problem
- [ -n $leasefile ] [ -e $leasefile ] || touch $leasefile
+ [ -n $leasefile ] ( [ -e $leasefile ] || touch $leasefile )
please apply
with kind regard
Peter
diff --git a/package/dnsmasq/files/dnsmasq.init b/package/dnsmasq/files/dnsmasq.init
index 3e194af..7b6f515 100644
--- a/package/dnsmasq/files/dnsmasq.init
+++ b/package/dnsmasq/files/dnsmasq.init
@@ -103,7 +103,7 @@ dnsmasq() {
[ $readethers = 1 ] [ -e /etc/ethers ] || touch /etc/ethers
config_get leasefile $cfg leasefile
- [ -n $leasefile ] [ -e $leasefile ] || touch $leasefile
+ [ -n $leasefile ] ( [ -e $leasefile ] || touch $leasefile )
config_get_bool cachelocal $cfg cachelocal 1
config_get hostsfile $cfg dhcphostsfile
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
