[U-Boot] [PATCH] cmd_fdt.c: fix parse of byte streams and strings

[Jerry Van Baren]

Ken MacLeod wrote:
> On Fri, Sep 11, 2009 at 11:30 AM, Jerry Van Baren 
> <gerald.vanbaren at ge.com <mailto:gerald.vanbaren at ge.com>> wrote:
> 
>     Scott Wood wrote:
>      > On Thu, Sep 10, 2009 at 08:23:27PM -0400, Jerry Van Baren wrote:
>      >>    fdt set /ethernet at f00 interrupts "this is a string"
>      >> can now handle multiple strings (words) by concatenating them with
>      >> spaces (quoted strings still work the same as before because of
>     hush's
>      >> argument parsing)
>      >>    fdt set /ethernet at f00 interrupts this is a string
>      >
>      > How do you set a string list, then?
> 
> The original code did not support string lists and this patch does not 
> address string lists.

The limitations of my original parsing shouldn't be taken as valid. ;-)

Scott has a very good point: the string parsing should take multiple 
(quoted) strings and turn them into proper string lists.  This is useful 
for things like compatibility lists.

>     I'm more concerned with the [] form because that really is a syntax
>     change.  The original syntax with a single quoted argument will no
>     longer be parsed if I understand the change (I need to apply the patch
>     and confirm this):
>     Old:
>        fdt set /ethernet at f00 interrupts "[33 2 34 2 36 2]"
>     becomes
>        fdt set /ethernet at f00 interrupts [ 33 2 34 2 36 2 ]
>     Note that the *must* be a space between "[" and "33" and between "2" and
>     "]" because the "[" and "]" now have to be separate arguments.  This is
>     what Andy did with "<" and ">" with no public outcry, so it is
>     probably OK.
> 
>     --------------------------------------------------------------
>     ==== Does anybody have a problem with this syntax change? ====
>     --------------------------------------------------------------
> 
> 
> There is no change in syntax as far as I can tell, it should parse byte 
> strings the same as one argument with spaces or as multiple arguments.  
> There remains a side effect (bug?) that if the the '[' and the next 

Bug. :-(

> value are separate arguments, a 0x00 gets inserted into the data.  The 
> original code either didn't parse the complete byte list (incrementing 
> stridx early) or hung in an endless loop.  This patch fixes that case.

OK, you made it better.  :-)

> The fix on strings is that in the original code if there were multiple 
> arguments then only the last argument was stored, at least now it stores 
> multiple arguments (collapsing inter-argument space, if any).

I agree with Scott, this is not good.  We should create lists.  If a 
user types
   fdt set /ethernet at f00 interrupts this is a string
he probably won't get what he wanted, but he will get what he deserves[1]:
   "this","is","a","string"
i.e. a list of four strings.

> I don't have Hush enabled right now and the non-Hush quoting rules are 
> still a little fuzzy for me right now.  The 'fdt set' wasn't working at 
> all for me without this patch.

I have not run with non-Hush in quite a while.  Sounds like I need to. 
The quoting rules should be the same.  Ahhh, you need to quote with 
apostrophes ' not double-quotes ".
   <http://www.denx.de/wiki/DULG/CommandLineParsing>
I don't know if spaces can be escaped with back-slashes off-hand, the 
page is silent on that.

>   -- Ken

Thanks,
gvb

[1]<http://www.lyricsdomain.com/18/rolling_stones/you_cant_always_get_what_you_want.html>

And I went down to the corporation
To get my fair share of abuse
Singing, "We're gonna vent our frustration
If we don't we're gonna blow a 50-amp fuse"
Sing it to me now...

You can't always get what you want
You can't always get what you want
You can't always get what you want
But if you try sometimes well you just might find
You get what you need

(yes, I changed one word)