[arch-projects] [initscripts] Incorrect environment variable checks in sysinit & co?
I just revisited [1] and found out that it isn't correct. The kernel only exports foo=bar pairs to init (and only if it doesn't know the left hand side, for instance, it doesn't export root=). Statements without = are not exported. The reason this worked is that mkinitcpio used 'export' to export 'verbose=y' in this case. This is not true anymore for 0.6.9 (and future releases, as 0.6.9 has a bug related to this change). I guess [1] should be reverted, and the initscripts should be search for more such checks. [1] http://projects.archlinux.org/initscripts.git/commit/?id=3d9fbd69fbab72ccdbd...
On Fri, Apr 01, 2011 at 12:40:12AM +0200, Thomas Bächler wrote:
I just revisited [1] and found out that it isn't correct. The kernel only exports foo=bar pairs to init (and only if it doesn't know the left hand side, for instance, it doesn't export root=). Statements without = are not exported.
The reason this worked is that mkinitcpio used 'export' to export 'verbose=y' in this case. This is not true anymore for 0.6.9 (and future releases, as 0.6.9 has a bug related to this change).
I guess [1] should be reverted, and the initscripts should be search for more such checks.
[1] http://projects.archlinux.org/initscripts.git/commit/?id=3d9fbd69fbab72ccdbd...
Both approaches are flawed, in my opinion, because the code that was removed would catch 'verboseforthehellofit'. In lieu of using grep's entirely broken -w flag and \b regex markers, I suggest one of two approaches: 1) Word split /proc/cmdline into an array and use in_array from /etc/rc.d/functions: kern_cmdline=($(< /proc/cmdline)) if in_array "${kern_cmdline[@]}" verbose; then ... 2) Word split /proc/cmdline with printf and use grep -x: if printf "%s\n" $(</proc/cmdline) | grep x 'verbose'; then ... The first approach is nice because we're not calling externals (I believe the < acts as a builtin), but has the tradeoff of leaving an array that we don't use. The second approach is a little cleaner, but requires calling grep. I can't really pick a favorite here, but we shouldn't just do a revert because I don't believe that's a proper solution. dave
On 2011/4/1 Dave Reisner <d@falconindy.com> wrote:
1) Word split /proc/cmdline into an array and use in_array from /etc/rc.d/functions:
kern_cmdline=($(< /proc/cmdline)) if in_array "${kern_cmdline[@]}" verbose; then ...
2) Word split /proc/cmdline with printf and use grep -x:
if printf "%s\n" $(</proc/cmdline) | grep x 'verbose'; then ...
These look very strange. I would be more tempted to inline the in_array and say: /bin/dmesg -n 3 for cmdlinearg do [[ $cmdlinearg == "verbose" ]] && /bin/dmesg -n 8 done which seems more readable and less magical to me. Rémy.
On Fri, Apr 01, 2011 at 01:35:18AM +0200, Rémy Oudompheng wrote:
On 2011/4/1 Dave Reisner <d@falconindy.com> wrote:
1) Word split /proc/cmdline into an array and use in_array from /etc/rc.d/functions:
kern_cmdline=($(< /proc/cmdline)) if in_array "${kern_cmdline[@]}" verbose; then ...
2) Word split /proc/cmdline with printf and use grep -x:
if printf "%s\n" $(</proc/cmdline) | grep x 'verbose'; then ...
These look very strange. I would be more tempted to inline the in_array and say:
/bin/dmesg -n 3 for cmdlinearg do [[ $cmdlinearg == "verbose" ]] && /bin/dmesg -n 8 done
which seems more readable and less magical to me.
Rémy.
Sure, also valid. It occurred to me that the array isn't actually necessary, as you can just word split in passing to in_array but inlining here is fine too. one minor nit to pick with the innards of your loop... [[ $arg = "verbose" ]] && { /bin/dmesg -n 8; break; } dave p.s. particularly as of late, I've found that "readable" is highly subjective, particularly when it comes to Bash.
On Fri, Apr 1, 2011 at 1:54 AM, Dave Reisner <d@falconindy.com> wrote:
On Fri, Apr 01, 2011 at 01:35:18AM +0200, Rémy Oudompheng wrote:
On 2011/4/1 Dave Reisner <d@falconindy.com> wrote:
1) Word split /proc/cmdline into an array and use in_array from /etc/rc.d/functions:
kern_cmdline=($(< /proc/cmdline)) if in_array "${kern_cmdline[@]}" verbose; then ...
2) Word split /proc/cmdline with printf and use grep -x:
if printf "%s\n" $(</proc/cmdline) | grep x 'verbose'; then ...
These look very strange. I would be more tempted to inline the in_array and say:
/bin/dmesg -n 3 for cmdlinearg do [[ $cmdlinearg == "verbose" ]] && /bin/dmesg -n 8 done
which seems more readable and less magical to me.
Rémy.
Sure, also valid. It occurred to me that the array isn't actually necessary, as you can just word split in passing to in_array but inlining here is fine too.
one minor nit to pick with the innards of your loop...
[[ $arg = "verbose" ]] && { /bin/dmesg -n 8; break; }
dave
p.s. particularly as of late, I've found that "readable" is highly subjective, particularly when it comes to Bash.
I propose two patches (mutualy exclusive), taking code from dave and remy. PS: Thomas, i (currently) cannot send patch directly from my git. Gonna happen. -- Sébastien Luttringer www.seblu.net
On Fri, Apr 1, 2011 at 2:36 AM, Seblu <seblu@seblu.net> wrote:
On Fri, Apr 1, 2011 at 1:54 AM, Dave Reisner <d@falconindy.com> wrote:
On Fri, Apr 01, 2011 at 01:35:18AM +0200, Rémy Oudompheng wrote:
On 2011/4/1 Dave Reisner <d@falconindy.com> wrote:
1) Word split /proc/cmdline into an array and use in_array from /etc/rc.d/functions:
kern_cmdline=($(< /proc/cmdline)) if in_array "${kern_cmdline[@]}" verbose; then ...
2) Word split /proc/cmdline with printf and use grep -x:
if printf "%s\n" $(</proc/cmdline) | grep x 'verbose'; then ...
These look very strange. I would be more tempted to inline the in_array and say:
/bin/dmesg -n 3 for cmdlinearg do [[ $cmdlinearg == "verbose" ]] && /bin/dmesg -n 8 done
which seems more readable and less magical to me.
Rémy.
Sure, also valid. It occurred to me that the array isn't actually necessary, as you can just word split in passing to in_array but inlining here is fine too.
one minor nit to pick with the innards of your loop...
[[ $arg = "verbose" ]] && { /bin/dmesg -n 8; break; }
dave
p.s. particularly as of late, I've found that "readable" is highly subjective, particularly when it comes to Bash.
I propose two patches (mutualy exclusive), taking code from dave and remy.
PS: Thomas, i (currently) cannot send patch directly from my git. Gonna happen.
Sorry, little mistake in previous one. -- Sébastien Luttringer www.seblu.net
participants (4)
-
Dave Reisner
-
Rémy Oudompheng
-
Seblu
-
Thomas Bächler