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

[Dave Reisner]

On Thu, Apr 14, 2011 at 10:43:31PM +0200, Thomas Bächler wrote:
> This should properly handle all ugly characters in values,
> omit settings any forbidden variable names and take care
> of double-quoted values with spaces in them.
> 
> This should finally fix FS#23467, FS#22080 and FS#13900.
> ---
> 
> I screwed up the last fix for FS#23467 when trying to fix
> the other two. This should now handle all cases just fine,
> although it duplicates some of the work the kernel
> already does.
> 
> I tested it better than 0.6.10 and it seems okay. Can
> someone please proof-read this and tell me if it is
> good?

Sandbox testing says this doesn't work. I feed in a file containing:

  root=/dev/sda video="din:10 foo bar baz" quiet ro

and after parsing, i echo "root=$root" and "video=$video". i get...

  video="din:10 foo bar baz
  root=/dev/sda

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 also never got a response as to where you're seeing pollution from
using export over eval. Sorry, but eval sucks hard and I've put a lot of
effort into _removing_ it from wherever I can in Arch code. It's
_rarely_ used properly. On top of it all, I've never seen any variables
declared on the command line populated into userspace.

Am I insane here?

d

> 
>  init_functions |   58 +++++++++++++++++++++++++++++++++++--------------------
>  1 files changed, 37 insertions(+), 21 deletions(-)
> 
> diff --git a/init_functions b/init_functions
> index fd5bbb6..e29d696 100644
> --- a/init_functions
> +++ b/init_functions
> @@ -31,27 +31,43 @@ launch_interactive_shell() {
>  }
>  
>  parse_cmdline() {
> -    eval set -- $(cat /proc/cmdline)
> -    for cmd in "$@"; 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 :)
> -            *=*) rhs="$(echo "${cmd}" | cut -d= -f2-)"
> -                cmd="$(echo "${cmd}" | cut -d= -f1 | sed 's|\.|_|g')"
> -                cmd="$(echo "${cmd}" | sed 's|-|_|g')=\"${rhs}\""
> -                (echo "${cmd}" | grep -qe '^[0-9]') || eval "${cmd}"
> -                ;;
> -            *)  cmd="$(echo "${cmd}" | sed 's|\.|_|g')"
> -                cmd="$(echo "${cmd}" | sed 's|-|_|g')"
> -                (echo "${cmd}" | grep -qe '^[0-9]') || eval "${cmd}=y"
> -                ;;
> -        esac
> +    local w in_quotes lhs rhs
> +    in_quotes=0
> +    for w in $(cat /proc/cmdline); do
> +        if [ ${in_quotes} -eq 0 ]; then
> +            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" ;;
> +                # only export stuff that does work with ash :)
> +                *=*) rhs="$(echo "${w}" | cut -d= -f2-)"
> +                    lhs="$(echo "${w}" | cut -d= -f1 | sed 's|\.|_|g;s|-|_|g;')"
> +                    if [ "${rhs:0:1}" = "\"" ]; then
> +                        if [ "${rhs:$((${#rhs}-1))}" = "\"" ]; then
> +                            rhs="${rhs:1:$((${#rhs}-2))}"
> +                        else
> +                            in_quotes=1
> +                            continue
> +                        fi
> +                    fi
> +                    (echo "${lhs}" | grep -qe '^[0-9]' -e '[^a-zA-Z0-9_]') || eval ${lhs}=\${rhs}
> +                    ;;
> +                *)  lhs="$(echo "${w}" | sed 's|\.|_|g;s|-|_|g;')"
> +                    (echo "${lhs}" | grep -qe '^[0-9]' -e '[^a-zA-Z0-9_]') || eval ${lhs}=y
> +                    ;;
> +            esac
> +        else
> +            if [ "${w:$((${#w}-1))}" = "\"" ]; then
> +                rhs="${rhs} ${w:0:$((${#w}-1))}"
> +                in_quotes=0
> +                (echo "${lhs}" | grep -qe '^[0-9]' -e '[^a-zA-Z0-9_]') || eval ${lhs}=\${rhs}
> +            else
> +                rhs="${rhs} ${w}"
> +            fi
> +        fi
>      done
>  }
>  
> -- 
> 1.7.4.4
>