Currently, when you write something like: staticiproute="... you get: # /etc/init.d/staticroute restart * WARNING: you are stopping a boot service * Caching service dependencies ... /etc/init.d/../conf.d/staticroute: line 11: unexpected EOF while looking for matching `"' /etc/init.d/../conf.d/staticroute: line 12: syntax error: unexpected end of file [ ok ] * Stopping sshd ... [ ok ] For obvious reasons parsing errors in conf.d files should be treated as critical errors, not as "ok". Reproducible: Always
sys-apps/openrc-0.6.1-r1 Portage 2.2_rc67 (hardened/linux/ia64/10.0/server, gcc-4.4.4, glibc-2.11.2-r0, 2.6.32-hardened-r10 ia64) ================================================================= System uname: Linux-2.6.32-hardened-r10-ia64-31-with-gentoo-2.0.1 Timestamp of tree: Wed, 14 Jul 2010 22:15:01 +0000 app-shells/bash: 4.0_p37 dev-lang/python: 2.6.4-r1, 3.1.2-r4 dev-util/cmake: 2.6.4-r3 sys-apps/baselayout: 2.0.1 sys-apps/openrc: 0.6.1-r1 sys-apps/sandbox: 2.2 sys-devel/autoconf: 2.65 sys-devel/automake: 1.10.3, 1.11.1 sys-devel/binutils: 2.20.1-r1 sys-devel/gcc: 4.4.4-r1 sys-devel/gcc-config: 1.4.1 sys-devel/libtool: 2.2.6b virtual/os-headers: 2.6.30-r1 ACCEPT_KEYWORDS="ia64" ACCEPT_LICENSE="* -@EULA" CBUILD="ia64-unknown-linux-gnu" CFLAGS="-pipe -mtune=mckinley -O2 -ftree-vectorize" CHOST="ia64-unknown-linux-gnu" CONFIG_PROTECT="/etc" CONFIG_PROTECT_MASK="/etc/ca-certificates.conf /etc/env.d /etc/fonts/fonts.conf /etc/gconf /etc/gentoo-release /etc/php/apache2-php5/ext-active/ /etc/php/cgi-php5/ext-active/ /etc/php/cli-php5/ext-active/ /etc/revdep-rebuild /etc/sandbox.d /etc/terminfo /etc/texmf/language.dat.d /etc/texmf/language.def.d /etc/texmf/updmap.d /etc/texmf/web2c /etc/udev/rules.d" CXXFLAGS="-pipe -mtune=mckinley -O2 -ftree-vectorize" DISTDIR="/var/cache/portage/distfiles" EMERGE_DEFAULT_OPTS="--with-bdeps y" FEATURES="assume-digests distlocks fixpackages news parallel-fetch preserve-libs protect-owned sandbox sfperms strict unmerge-logs unmerge-orphans userfetch userpriv usersandbox usersync" GENTOO_MIRRORS="http://ftp.spline.inf.fu-berlin.de/mirrors/gentoo/ http://ftp-stud.fht-esslingen.de/pub/Mirrors/gentoo/ http://distfiles.gentoo.org" LANG="en_GB.UTF-8" LDFLAGS="-Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed" MAKEOPTS="-j3" PKGDIR="/var/cache/portage/packages" PORTAGE_COMPRESS="xz" PORTAGE_CONFIGROOT="/" PORTAGE_RSYNC_EXTRA_OPTS=" --include='/sci-libs/' --include='/sci-libs/gsl/' --exclude='/sci-libs/*/' --include='/x11-libs/' --include='/x11-libs/qt*/' --include='/x11-libs/cairo/' --include='/x11-libs/pango/' --include='/x11-libs/pixman/' --exclude='/x11-libs/*/' --include='/x11-misc/' --include='/x11-misc/util-macros/' --exclude='/x11-misc/*/' --exclude='/games*/' --exclude='/gnome*/' --exclude='/gnustep*/' --exclude='/gpe*/' --exclude='/kde*/' --exclude='/lxde*/' --exclude='/rox*/' --exclude='/sci*/' --exclude='/x11*/' --exclude='/xfce*/'" PORTAGE_RSYNC_OPTS="--recursive --links --safe-links --perms --times --compress --force --whole-file --delete --stats --timeout=180 --exclude=/distfiles --exclude=/local --exclude=/packages" PORTAGE_TMPDIR="/var/tmp" PORTDIR="/var/cache/portage/gentoo" PORTDIR_OVERLAY="/var/cache/portage/layman/hardened-development /var/cache/portage/layman/sunrise /var/cache/portage/layman/anarchy /var/cache/portage/local" SYNC="rsync://rsync.europe.gentoo.org/gentoo-portage" [...] Unset: CPPFLAGS, CTARGET, FFLAGS, INSTALL_MASK, LC_ALL, LINGUAS, PORTAGE_COMPRESS_FLAGS
I think this applies to all conf.d files, not just network related files. The issue, as I understand it, is that when one of these files is sourced, we continue processing the service regardless of whether or not the file was sourced successfully.
All, I have a proposed patch to fix this issue, but, before I commit this to the tree, I want to discuss it here. My patch makes runscript.sh check the return code from the "." commands which are used to source the conf.d files, /etc/init.d/functions.sh and /lib/rc/sh/rc-functions.sh. When I generated the type of errors described in the initial description of this bug, I saw a message go by that said that future versions of the shell will abort for errors in . and eval commands to fulfill a posix requirement. Once that happens,, if this fix is applied, there will be code in runscript.sh which will never be executed. I will attach my proposed patch below. Comments are welcome.
Created attachment 259329 [details] 0001-bug-328675-add-error-checking-to-runscript.sh.patch This is my proposal for fixing this issue.
you should be able to inline source commands: if ! . /foo ; then eerror "$RC_SVCNAME: error loading /foo" exit 1 fi although to take this a bit further, how about we define a new "sourcex" func: sourcex() { if ! . "$1" ; then eerror "$RC_SVCNAME: error loading $1" exit 1 fi }
Created attachment 259630 [details] 0001-bug-328675-add-error-checking-to-runscript.sh.patch All, this is my revised patch which incorporates the feedback I have received from Mike. If there are no more suggestions, I will push this to the master branch of the repository around 0200 UTC on 1/13. Thanks, William
looks good ... only minor style issue: -[ -e @SYSCONFDIR@/rc.conf ] && . @SYSCONFDIR@/rc.conf +if [ -e @SYSCONFDIR@/rc.conf ]; then + sourcex "@SYSCONFDIR@/rc.conf" +fi that could remain: [ -e @SYSCONFDIR@/rc.conf ] && sourcex "@SYSCONFDIR@/rc.conf" although, i see a bunch of places where we have this same kind of logic. so we could tweak that too like so: sourcex() { if [ "$1" = "-e" ] ; then shift [ -e "$1" ] || return 1 fi if ! . "$1"; then eerror "$RC_SVCNAME: error loading $1" exit 1 fi } and that code could turn into: sourcex -e "@SYSCONFDIR@/rc.conf" and in earlier code, we could turn this: if [ -e "$_conf_d/$_c.$RC_RUNLEVEL" ]; then sourcex "$_conf_d/$_c.$RC_RUNLEVEL" elif [ -e "$_conf_d/$_c" ]; then sourcex "$_conf_d/$_c" fi into: if ! sourcex -e "$_conf_d/$_c.$RC_RUNLEVEL" ; then sourcex -e "$_conf_d/$_c" fi
This has been added to git, commit 84eda6.
