Re: [PATCH] dracut-lib: do not use cached CMDLINE in _getcmdline

[Harald Hoyer <harald-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On 09/04/2013 06:10 PM, WANG Chao wrote:
> Cached CMDLINE doesn't work 100%. For example the following case,
> 
> 1. dracut starts to run dracut-cmdline.sh. CMDLINE is cached when calling
>    getarg 'rd.break=cmdline'.
> 2. In 92-parse-ibft.sh, ibft_to_cmdline() calls $(set_ifname ibft xx:xx..)
>    multiple times in each subshell.
> 3. In 1st call, set_ifname() will check $(getargs ifname) and write out
>    ifname=xxxx accordingly.
> 4. In 2nd call, set_ifname() will check $(getargs ifname) and it's wrong here.
>    Because in step 3, we introduce a new cmdline arg ifname=xxx, but CMDLINE
>    isn't updated. Thus we fail to get the new ifname arg.
> 
> It's doable to unset CMDLINE every time after a new cmdline arg is in. But
> unset should be done in the parent process, because unset CMDLINE in a
> subshell won't unset CMDLINE in its parent or sibling process. And also it's
> painful to unset CMDLINE every time. In the future, functions and code
> snippet could probably separate or move to other file, the unset CMDLINE could
> malfunction again like this time.
> 
> So I'm thinking not to cache CMDLINE. It's doesn't hurt to re-read all the
> cmdline args everytime. Because it's in initramfs, a non cached _getcmdline()
> should be fast enough.
> 
> Please consider!
> 
> Thanks
> WANG Chao
> ---
>  modules.d/99base/dracut-lib.sh | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/modules.d/99base/dracut-lib.sh b/modules.d/99base/dracut-lib.sh
> index 0d5cab9..ef8c5a3 100755
> --- a/modules.d/99base/dracut-lib.sh
> +++ b/modules.d/99base/dracut-lib.sh
> @@ -103,23 +103,22 @@ _getcmdline() {
>      local _line
>      local _i
>      unset _line
> -    if [ -z "$CMDLINE" ]; then
> -        unset CMDLINE_ETC CMDLINE_ETC_D
> -        if [ -e /etc/cmdline ]; then
> -            while read -r _line; do
> -                CMDLINE_ETC="$CMDLINE_ETC $_line";
> -            done </etc/cmdline;
> -        fi
> -        for _i in /etc/cmdline.d/*.conf; do
> -            [ -e "$_i" ] || continue
> -            while read -r _line; do
> -                CMDLINE_ETC_D="$CMDLINE_ETC_D $_line";
> -            done <"$_i";
> -        done
> -        if [ -e /proc/cmdline ]; then
> -            read -r CMDLINE </proc/cmdline;
> -            CMDLINE="$CMDLINE_ETC_D $CMDLINE_ETC $CMDLINE"
> -        fi
> +    unset CMDLINE_ETC CMDLINE_ETC_D
> +
> +    if [ -e /etc/cmdline ]; then
> +        while read -r _line; do
> +            CMDLINE_ETC="$CMDLINE_ETC $_line";
> +        done </etc/cmdline;
> +    fi
> +    for _i in /etc/cmdline.d/*.conf; do
> +        [ -e "$_i" ] || continue
> +        while read -r _line; do
> +            CMDLINE_ETC_D="$CMDLINE_ETC_D $_line";
> +        done <"$_i";
> +    done
> +    if [ -e /proc/cmdline ]; then
> +        read -r CMDLINE </proc/cmdline;
> +        CMDLINE="$CMDLINE_ETC_D $CMDLINE_ETC $CMDLINE"
>      fi
>  }
>  
> 

Yes, you are right! Will merge! Thanks!!