[arch-projects] [mkinitcpio] [RFC] Rewrite parse_cmdline (again)

[Dave Reisner]

On Fri, Apr 15, 2011 at 12:13:29AM +0200, Rémy Oudompheng wrote:
> On Thu 14 April 2011 at 17:44 -0400, dave reisner wrote:
> > On Apr 14, 2011 5:28 PM, "Thomas Bächler" <thomas at archlinux.org> wrote:
> > > > I posted what I thought was a valid solution on FS#23467 from stack
> > > > overflow that seems to be a lot more sane and _much_ more maintainable.
> > > > For those who weren't following the report, I suggested that we use the
> > > > same fallback as /etc/fstab, which says to encode spaces with octal
> > > > sequences. These can then be decoded using printf's %b flag. It requires
> > > > only a simple change to the current parse_cmdline function.
> > >
> > > I really don't understand what you mean (or how it would help). What you
> > > mentioned doesn't solve the initial problem of finding out which spaces
> > > separate arguments and which spaces are inside quoted strings.
> > >
> > 
> > It absolutely does because it eliminates spaces within variables, e.g.
> > video=foo\040bar root=…
> > 
> > The whole point is that quotes aren't used. fstab sets precedent here so its
> > not some wild and whacky new thing being introduced.
> 
> This looks very nice, but how do we get a space-escaped kernel command
> line? Is the user expected to encode his command line at boot time?
> 
> And do we really have to filter out invalid variable names? Isn't there
> some documentation that says the kernel isn't supposed to have absurd
> command line parameters (like unescaped >, <, &, |)
> 
> Because I think that (minus eval statements), the function would look
> better like this :
> 
> local w lhs rhs
> eval set -- $(cat test)
> for w; do
>     case "${w}" in
>         \#*) break ;; # ignore everything after a # in the commandline
>         # The kernel passes those to init on its own
>         [0123456Ss]) ;;
>         single) ;;
>         rw) readwrite="yes" ;;
>         ro) readwrite="no" ;;
>         # variables don't begin with a digit
>         [0-9]*) ;;
>         # only export stuff that does work with ash :)
>         *=*)
>             lhs=${w%%=*}
>             rhs=${w#*=}
>             # replace forbidden characters
>             lhs=${lhs/./_}
>             lhs=${lhs/-/_}
>             eval "${lhs}='${rhs}'"
>             ;;
>         *)
>             lhs=${w/./_}
>             lhs=${w/-/_}
>             eval "${lhs}=y"
>             ;;
>     esac
> done
> 
> -- 
> Rémy.

Yes, the idea is that the user is expected to escape their own
whitespace, just as they are expected to do so in /etc/fstab. I tested
the following across the open bug reports and can report great success
on all fronts:

parse_cmdline() {
  for cmd in $(cat /proc/cmdline); do
    case $cmd in
      \#*) break ;; # ignore everything after a # in the commandline
      # The kernel passes those to the kernel on its own
      [0123456Ss]) ;;
      [0-9]*) ;;
      single) ;;
      rw) readwrite="yes" ;;
      ro) readwrite="no" ;;
      # only export stuff that does work with ash :)
      *=*) lhs=${cmd%%=*}
           lhs=${lhs//[-.]/_}
           rhs=$(printf '%b' "${cmd#*=}")
           export "$lhs"="$rhs"
           ;;
      *) cmd=${cmd//[-.]/_}
         export "$cmd"="y"
         ;;
    esac
  done
}

Yes, I've also removed the grep checks prior to each export. They're
redundant because of the 3rd case down, [0-9]*, already checking for a
variable starting with a leading digit.

Thomas mentioned in IRC that there was a "huge problem" with this, but
had to go to sleep. I'm interested in what he has to say.

dave