barebox-state (dt-utils): dump-shell broken for state at 0

[Marc Kleine-Budde]

On 04/05/2016 08:49 AM, Sascha Hauer wrote:
> +Cc mkl who wrote 6d58ca4
> 
> On Mon, Apr 04, 2016 at 11:44:51PM +0200, Andreas Pretzsch wrote:
>> On Mi, 2015-06-17 at 11:06 +0200, Sascha Hauer wrote:
>>> On Wed, Jun 17, 2015 at 08:42:50AM +0200, Jan Remmet wrote:
>>>> Hello,
>>>> I'm working with barebox states and wonder if there is already a tool to access
>>>> states in eeprom from linux?
>>>> The states from a dtb backend should be easy to read. For raw there must be
>>>> looked under /proc/devicetree or a state kernel driver.
>>>
>>> Yes, there is such a tool here:
>>>
>>> git://git.pengutronix.de/git/tools/dt-utils.git
>>>
>>> The binary you'll need is barebox-state.
>>
>> barebox-state provides a command "--dump-shell" to dump the state
>> suitable for shell sourcing.
>> Essentially, it prints out variable=value pairs.
>> So typical use would be a
>> 	eval `barebox-state --dump-shell`
>> somewhere in a shell script (incl. simple ones like busybox ash).
>>
>> Unfortunately, this breaks with indices in the state name, as @ is not
>> escaped.
>> Tested with dt-utils v2015.10.0, but according to code this is the case
>> from v2015.05.0 up until master.
>>
>> As of commit 6d58ca4 "barebox-state: fix export of shell variables:",
>> the fixed prefix "STATE_" was replaced by the supplied state name.
>> Also in there, all '.' are replaced by '_' in the variable name. Not in
>> the state name itself.
>>
>> The same would be necessary for (at least) '@', because it is invalid
>> also inside a shell variable name. As probably other chars, just I'm not
>> sure which might show up from the dts. So no premature patch from my
>> side.
>> Not only for the variable name, but also the state name itself.

I think the "@" is removed from the state name, isn't it? See
"state_convert_node_variable()":

>         /* strip trailing @<ADDRESS> */
>         short_name = xstrdup(node->name);
>         indexs = strchr(short_name, '@');
>         if (indexs)
>                 *indexs = 0;

>>
>> For clarification of the setup and behaviour, see below.
>>
>>
>> Now, question would be how to fix this.
>> Also replacing '@' by '_' might break existing users (e.g. when parsing
>> from stdin or similar, instead of using shells eval command).
>> On the other hand, status quo breaks the "described" use of dump
>> _shell_ ...
>>
>> Talking about this, it might be also some idea to resurrect the old
>> behaviour of always printing 'STATE_' as prefix instead of the state
>> name from dts. Not sure if it's the best idea, or how to call such an
>> extra option, didn't think it through by now.
>> Just saying, as I will go this way here (as a hotfix) for my setup...
>>
>> Ideas, opinions ?
> 
> I'm in favor for resurrecting the old 'STATE_' behaviour, but I don't
> know why Marc changed it. Maybe he has some more input.

We make use of more than one state partition in one shell script.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/barebox/attachments/20160406/aff50b09/attachment.sig>