[OE-core] [PATCH v2] Introduce new SERIAL_CONSOLES to add multiple consoles for your MACHINE

[McClintock Matthew-B29882]

On Fri, Nov 4, 2011 at 4:34 PM, Phil Blundell <philb at gnu.org> wrote:
>> --- a/meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb
>> +++ b/meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb
>> @@ -25,6 +25,18 @@ do_install() {
>>      if [ ! -z "${SERIAL_CONSOLE}" ]; then
>>          echo "S:2345:respawn:${base_sbindir}/getty ${SERIAL_CONSOLE}" >> ${D}${sysconfdir}/inittab
>>      fi
>> +
>> +    idx=0
>> +    tmp="${SERIAL_CONSOLES}"
>> +    if [ "x" != "x$tmp" ]; then
>> +     for i in $tmp
>> +     do
>> +         j=`echo ${i} | sed s/\;/\ /g`
>> +         echo "${idx}:2345:respawn:${base_sbindir}/getty ${j}" >> ${D}${sysconfdir}/inittab
>> +         idx=`expr $idx + 1`
>> +     done
>> +    fi
>> +
>
> Did you test that this actually solves the problem?  It looks to me as
> though it will still do the wrong thing if the variable is not defined
> at all (as opposed to being defined to the empty string).  The added
> "if" statement doesn't actually achieve much since, if $tmp is the empty
> string, the for-loop wouldn't have executed anyway and there is no need
> for an extra guard.
>
> Also, for what it's worth, the "x$tmp" idiom is unnecessary if you're
> going to quote the strings.

I don't get it. I think the first version was still OK. The first
version is pasted below.

> +    idx=0
> +    tmp="${SERIAL_CONSOLES}" <- If it's not defined this will catch it.
> +    for i in $tmp
> +    do
> +     j=`echo ${i} | sed s/\;/\ /g`
> +        echo "${idx}:2345:respawn:${base_sbindir}/getty ${j}" >> ${D}${sysconfdir}/inittab
> +     idx=`expr $idx + 1`
> +    done

See my inline comment above, but if SERIAL_CONSOLES is not defined
tmp="" and the for loop won't execute. Not sure if there is a more
standard way to do this though.

-M