Date: Wed, 15 Jun 2011 23:02:55 +0200 From: Jilles Tjoelker <jilles@stack.nl> To: jhell <jhell@DataIX.net> Cc: freebsd-rc@freebsd.org Subject: Re: [PATCH] Knock out two if statements, one eval & IDCMD with builtin test. Message-ID: <20110615210255.GA44402@stack.nl> In-Reply-To: <20110615034526.GA12185@DataIX.net> References: <20110615034526.GA12185@DataIX.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Jun 14, 2011 at 11:45:26PM -0400, jhell wrote:
> After looking over Jilles patch on this same list it made ID & IDCMD
> catch my eye when I seen the $(eval $IDCMD) where it was the only place
> it was used throughout the whole system in which it calls another if
> statement from IDCMD to check the presence of /usr/bin/id.
> This is not bad at all, don't get me wrong but this could be done from
> one location to knock out the eval and two if statements with one
> builtin test right from the ID variable itself and get rid of the need
> for the IDCMD.
> Slight speed improvement ? maybe... cleaner yes.
There is no patch included. Maybe you forgot it or the mailing list
software ate it?
> As for functionality can anyone think of a need to wait for processing
> this till run_rc_command is thrown ? if so should it be escaped and
> re-eval'd as $(eval \$ID) or something similiar later ?
It is useful to postpone the check because it usually is not necessary
at all. The id command only needs to be executed if ${name}_user is set.
Also note that when /etc/rc.subr is sourced for the first time, /usr may
not be mounted. Therefore the unavailability of /usr/bin/id must not be
cached.
As for performance, expanding "$(eval $IDCMD)" currently takes two forks
(assuming /usr/bin/id is executable), which can be reduced to one with a
simple tweak to sh (only for 9.x sh though, not 8.x). The second fork
can also be avoided by rearranging the script, just using "$($ID)"
and checking its existence outside command substitution.
Index: bin/sh/eval.c
===================================================================
--- bin/sh/eval.c (revision 223024)
+++ bin/sh/eval.c (working copy)
@@ -140,7 +140,7 @@
STPUTC('\0', concat);
p = grabstackstr(concat);
}
- evalstring(p, builtin_flags & EV_TESTED);
+ evalstring(p, builtin_flags);
} else
exitstatus = 0;
return exitstatus;
@@ -908,6 +908,7 @@
dup2(pip[1], 1);
close(pip[1]);
}
+ flags &= ~EV_BACKCMD;
}
flags |= EV_EXIT;
}
--
Jilles Tjoelker
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110615210255.GA44402>