[arch-projects] [initscripts][PATCH 0/7] Cleanup and new feature

newer
[arch-projects] [initscripts] next...

Dave Reisner

7 Nov 2011 7 Nov '11

1:27 a.m.

Hi all, This fixes a couple of items that have been nagging at me, the largest of which being that we parse environment files such as /etc/locale.conf. This is pretty bad, imo, and we shouldn't be doing it. Benefits of not doing it are laid out in the patch that implements it. Tom, I'm not sure when you planned on doing a release, but hopefully some of these are small enough that you won't mind merging them. d Dave Reisner (7): avoid unnecessary escaping of newlines fix non-uniform indentation functions: remove redundant 'return $?' functions: properly quote ${mounts[@]} functions: declare locale vars in an array for reuse functions: implement a environment file parser PKGBUILD: use %Y%m%d format instead of %s PKGBUILD | 2 +- functions | 70 ++++++++++++++++++++++++++++++++++++++++++++++++---------- rc.shutdown | 2 +- rc.sysinit | 16 ++++++------ 4 files changed, 68 insertions(+), 22 deletions(-) -- 1.7.7.2

Show replies by date

Dave Reisner

7 Nov 7 Nov

1:27 a.m.

New subject: [arch-projects] [initscripts][PATCH 1/7] avoid unnecessary escaping of newlines

The bash parser assumes that an expression continues when || is at the end of a line. Signed-off-by: Dave Reisner --- rc.sysinit | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/rc.sysinit b/rc.sysinit index 1c74bd9..eb66935 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -15,14 +15,14 @@ printsep mountpoint -q /proc || mount -n -t proc proc /proc -o nosuid,noexec,nodev mountpoint -q /sys || mount -n -t sysfs sys /sys -o nosuid,noexec,nodev mountpoint -q /run || mount -n -t tmpfs run /run -o mode=0755,size=10M,nosuid,nodev -mountpoint -q /dev || mount -n -t devtmpfs udev /dev -o mode=0755,size=10M,nosuid \ - || mount -n -t tmpfs udev /dev -o mode=0755,size=10M,nosuid +mountpoint -q /dev || mount -n -t devtmpfs udev /dev -o mode=0755,size=10M,nosuid || + mount -n -t tmpfs udev /dev -o mode=0755,size=10M,nosuid mkdir -p -m 1777 /run/lock mkdir -p /dev/{pts,shm} -mountpoint -q /dev/pts || mount -n /dev/pts &>/dev/null \ - || mount -n -t devpts devpts /dev/pts -o mode=0620,gid=5,nosuid,noexec -mountpoint -q /dev/shm || mount -n /dev/shm &>/dev/null \ - || mount -n -t tmpfs shm /dev/shm -o mode=1777,nosuid,nodev +mountpoint -q /dev/pts || mount -n /dev/pts &>/dev/null || + mount -n -t devpts devpts /dev/pts -o mode=0620,gid=5,nosuid,noexec +mountpoint -q /dev/shm || mount -n /dev/shm &>/dev/null || + mount -n -t tmpfs shm /dev/shm -o mode=1777,nosuid,nodev # remount root ro to allow for fsck later on, we remount now to # make sure nothing can open files rw on root which would block a remount -- 1.7.7.2

Dave Reisner

1:27 a.m.

New subject: [arch-projects] [initscripts][PATCH 2/7] fix non-uniform indentation

Signed-off-by: Dave Reisner --- functions | 2 +- rc.shutdown | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/functions b/functions index 81328df..471c7d1 100644 --- a/functions +++ b/functions @@ -370,7 +370,7 @@ activate_vgs() { # Newer packages will only have /sbin/cryptsetup and no static binary # This ensures maximal compatibility with the old and new layout for CS in /sbin/cryptsetup /usr/sbin/cryptsetup \ - /sbin/cryptsetup.static ''; do + /sbin/cryptsetup.static ''; do [[ -x $CS ]] && break done diff --git a/rc.shutdown b/rc.shutdown index 86ff799..30708ce 100755 --- a/rc.shutdown +++ b/rc.shutdown @@ -121,7 +121,7 @@ if [[ -x /run/initramfs/shutdown ]]; then else status "Remounting Root Filesystem Read-only" \ - mount -n -o remount,ro / + mount -n -o remount,ro / # Power off or reboot printsep -- 1.7.7.2

Dave Reisner

1:27 a.m.

New subject: [arch-projects] [initscripts][PATCH 3/7] functions: remove redundant 'return $?'

Signed-off-by: Dave Reisner --- functions | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/functions b/functions index 471c7d1..d388260 100644 --- a/functions +++ b/functions @@ -418,7 +418,6 @@ NETFS="nfs,nfs4,smbfs,cifs,codafs,ncpfs,shfs,fuse,fuseblk,glusterfs,davfs,fuse.g # Check local filesystems fsck_all() { fsck -A -T -C"$FSCK_FD" -a -t "no${NETFS//,/,no},noopts=_netdev" $FORCEFSCK - return $? } # Single-user login and/or automatic reboot after fsck (if needed) -- 1.7.7.2

Dave Reisner

1:27 a.m.

New subject: [arch-projects] [initscripts][PATCH 4/7] functions: properly quote ${mounts[@]}

Signed-off-by: Dave Reisner --- This one is fairly important... functions | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/functions b/functions index d388260..7de58c1 100644 --- a/functions +++ b/functions @@ -485,7 +485,7 @@ umount_all() { mounts+=("$target") done < <(findmnt -runRo TARGET,FSTYPE,OPTIONS / | tac) - umount -r ${mounts[@]} + umount -r "${mounts[@]}" } -- 1.7.7.2

Dave Reisner

1:27 a.m.

New subject: [arch-projects] [initscripts][PATCH 5/7] functions: declare locale vars in an array for reuse

This also fixes a bug that unintentionally sets LOCALE instead of just defaulting to a value when its empty. Signed-off-by: Dave Reisner --- functions | 17 +++++++++-------- 1 files changed, 9 insertions(+), 8 deletions(-) diff --git a/functions b/functions index 7de58c1..ce664ed 100644 --- a/functions +++ b/functions @@ -5,6 +5,10 @@ # sanitize PATH (will be overridden later when /etc/profile is sourced, but is useful for UDev) export PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" +localevars=(LANG LC_CTYPE LC_NUMERIC LC_TIME LC_COLLATE LC_MONETARY + LC_MESSAGES LC_PAPER LC_NAME LC_ADDRESS LC_TELEPHONE + LC_MEASUREMENT LC_IDENTIFICATION LC_ALL) + if [[ $1 == "start" ]]; then if [[ $STARTING ]]; then echo "A daemon is starting another daemon, this is unlikely to work as intended." @@ -64,17 +68,14 @@ unset TERM_COLORS unset TZ # sanitize the locale settins -unset LC_CTYPE LC_NUMERIC LC_TIME LC_COLLATE LC_MONETARY \ - LC_MESSAGES LC_PAPER LC_NAME LC_ADDRESS LC_TELEPHONE \ - LC_MEASUREMENT LC_IDENTIFICATION LC_ALL +unset "${localevars[@]}" + if [[ $DAEMON_LOCALE = [yY][eE][sS] ]]; then - LANG="${LOCALE:=C}" - if [ -r /etc/locale.conf ]; then + LANG=${LOCALE:-C} + if [[ -r /etc/locale.conf ]]; then . /etc/locale.conf fi - export LANG LC_CTYPE LC_NUMERIC LC_TIME LC_COLLATE LC_MONETARY \ - LC_MESSAGES LC_PAPER LC_NAME LC_ADDRESS LC_TELEPHONE \ - LC_MEASUREMENT LC_IDENTIFICATION LC_ALL + export "${localevars[@]}" else export LANG=C fi -- 1.7.7.2

Dave Reisner

1:27 a.m.

New subject: [arch-projects] [initscripts][PATCH 6/7] functions: implement a environment file parser

This adds a 'parse_envfile' function that reads files such as /etc/locale.conf and /etc/vconsole.conf without sourcing them as bash logic. Several benefits are realized from this: - Impossible to execute arbitrary code - Bad syntax won't prevent the entire file from being read - Possible to limit what variables are allowed Signed-off-by: Dave Reisner --- functions | 50 ++++++++++++++++++++++++++++++++++++++++++++++++-- rc.sysinit | 4 ++-- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/functions b/functions index ce664ed..387f2af 100644 --- a/functions +++ b/functions @@ -9,6 +9,8 @@ localevars=(LANG LC_CTYPE LC_NUMERIC LC_TIME LC_COLLATE LC_MONETARY LC_MESSAGES LC_PAPER LC_NAME LC_ADDRESS LC_TELEPHONE LC_MEASUREMENT LC_IDENTIFICATION LC_ALL) +vconsolevars=(KEYMAP KEYMAP_TOGGLE FONT FONT_MAP FONT_UNIMAP) + if [[ $1 == "start" ]]; then if [[ $STARTING ]]; then echo "A daemon is starting another daemon, this is unlikely to work as intended." @@ -70,12 +72,56 @@ unset TZ # sanitize the locale settins unset "${localevars[@]}" +parse_envfile() { + local file=$1 validkeys=${@:2} ret=0 lineno=0 key= val= + + if [[ -z $file ]]; then + printf "error: no environment file specified\n" + return 1 + fi + + if [[ ! -f $file ]]; then + printf "error: cannot parse \`%s': No such file or directory\n" "$file" + return 1 + fi + + if [[ ! -r $file ]]; then + printf "error: cannot read \`%s': Permission denied\n" "$file" + return 1 + fi + + while IFS='=' read -r key val; do + (( ++lineno )) + + # trim whitespace, avoiding usage of a tempfile + key=$(echo "$1" | { read -r val; echo "$val"; }) + val=$(echo "$1" | { read -r val; echo "$val"; }) + + [[ $key ]] || continue + + if [[ -z $val ]]; then + printf "error: found key \`%s' without value on line %s of %s\n" \ + "$key" "$lineno" "$file" + (( ++ret )) + continue + fi + + # ignore invalid keys if we have a list of valid ones + if (( ${#validkeys[*]} )); then + in_array "$key" "${validkeys[@]}" || continue + fi + + export "$key=$val" || (( ++ret )) + done <"$file" + + return $ret +} + if [[ $DAEMON_LOCALE = [yY][eE][sS] ]]; then LANG=${LOCALE:-C} if [[ -r /etc/locale.conf ]]; then - . /etc/locale.conf + parse_envfile /etc/locale.conf "${localevars[@]}" fi - export "${localevars[@]}" else export LANG=C fi diff --git a/rc.sysinit b/rc.sysinit index eb66935..654a409 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -237,7 +237,7 @@ if [[ $HOSTNAME ]]; then fi if [[ -s /etc/locale.conf ]]; then - . /etc/locale.conf + parse_envfile /etc/locale.conf "LANG" [[ $LANG ]] && LOCALE=$LANG fi if [[ ${LOCALE,,} =~ utf ]]; then @@ -263,7 +263,7 @@ else fi if [[ -s /etc/vconsole.conf ]]; then - . /etc/vconsole.conf + parse_envfile /etc/vconsole.conf "${vconsolevars[@]}" [[ $FONT ]] && CONSOLEFONT=$FONT [[ $FONT_MAP ]] && CONSOLEMAP=$FONT_MAP fi -- 1.7.7.2

Tom Gundersen

1:39 a.m.

New subject: [arch-projects] [initscripts][PATCH 6/7] functions: implement a environment file parser

On Mon, Nov 7, 2011 at 11:27 AM, Dave Reisner wrote:

...

ds a 'parse_envfile' function that reads files such as /etc/locale.conf and /etc/vconsole.conf without sourcing them as bash logic. Several benefits are realized from this:

- Impossible to execute arbitrary code - Bad syntax won't prevent the entire file from being read - Possible to limit what variables are allowed

Awesome! -t

Dave Reisner

1:27 a.m.

New subject: [arch-projects] [initscripts][PATCH 7/7] PKGBUILD: use %Y%m%d format instead of %s

Using seconds from epoch will cause 'makepkg -i' to fail. Lame. Signed-off-by: Dave Reisner --- PKGBUILD | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/PKGBUILD b/PKGBUILD index deddbd5..2667db0 100644 --- a/PKGBUILD +++ b/PKGBUILD @@ -1,5 +1,5 @@ pkgname=initscripts-git -pkgver=$(date +%s) +pkgver=$(date +%Y%m%d) pkgrel=$(git log -1 --pretty=format:%h) pkgdesc="System initialization/bootup scripts" arch=('any') -- 1.7.7.2

Tom Gundersen

1:41 a.m.

On Mon, Nov 7, 2011 at 11:27 AM, Dave Reisner wrote:

...

Hi all,

This fixes a couple of items that have been nagging at me, the largest of which being that we parse environment files such as /etc/locale.conf. This is pretty bad, imo, and we shouldn't be doing it. Benefits of not doing it are laid out in the patch that implements it.

Tom, I'm not sure when you planned on doing a release, but hopefully some of these are small enough that you won't mind merging them.

I'll merge them all and delay the release. The reason being that your big patch (which I love) correctly restricts what can be in vconsole.conf and locale.conf. Since vconsole.conf is new and locale.conf is almost new, I think it makes sense to include this as soon as possible. Cheers, Tom

4559

Age (days ago)

4559

Last active (days ago)

List overview

Download

9 comments

2 participants

participants (2)

Dave Reisner
Tom Gundersen

[arch-projects] [initscripts][PATCH 0/7] Cleanup and new feature

tags

participants (2)