Re: [PATCH v2 3/3] dtc: Support character literals in bytestrings

[Anton Staaf <robotboy-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>]

On Sun, Sep 11, 2011 at 10:09 PM, Anton Staaf <robotboy-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> On Sun, Sep 11, 2011 at 5:48 PM, David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>> On Fri, Sep 09, 2011 at 11:37:51AM -0700, Anton Staaf wrote:
>>> On Fri, Sep 9, 2011 at 12:25 AM, David Gibson
>>> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>>> > On Thu, Sep 08, 2011 at 11:30:42PM -0700, Anton Staaf wrote:
>> [snip]
>>> > I supposed modifying your suggestion, but combining with our existing
>>> > convention for "reserved words" we could do:
>>> >
>>> >        prop = /uint8/ <0xab 0xcd>;
>>> >
>>> > I don't love it, but it's about the best I've come up with yet.  And
>>> > in particular it's probably the variant I'd be least upset to carry
>>> > around as legacy if we come up with a better way in future.
>>>
>>> Yes, I like this better than my suggestions of using !.  I wasn't sure
>>> if the /.../ syntax was something that was going to be allowed in
>>> property definitions.  One other option working from this could be:
>>>
>>> property = /size/ 8 <0xab 0xcd>;
>>
>> I quite like that idea.
>
> OK, I can take a look at writing up a patch to add this.  Unless it
> was something anyone else wanted to do.  :)
>
> -Anton

So, I've done some looking into an implementation of the /size/
syntax.  Here's what I'm thinking so far:

1) Parsing /size/ and the subsequent literal are easy, we knew this already.

2) The phandle references are a problem.  They are fixed up in
checks.c:fixup_phandle_references and that function expects that there
is 32-bits of space (sizeof(cell_t)) available to write the phandle
into.

3) Communicating the current size of the cells from the parser rule
that reads the DT_SIZE and DT_LITERAL to the parser rule that reads
the cellval requires either some auxiliary data or an explosion of the
parser rules (one per size we want to support).  Obviously the latter
is bad.

So, one solution to the above problems is to disallow phandle
references in anything other than a uint32 cell list, and to augment
the "data" struct to include the current cell size.  This can be used
to reject any attempts to add a REF_PHANDLE marker when the cell size
is not 32-bits.  The fixup code in checks.c would then never encounter
a REF_PHANDLE in a location that didn't have enough space reserved for
it.  A slight modification of the above could be to allow a
REF_PHANDLE any time the current cell size is greater than 32 bits.

Other solutions include always appending 32-bits and adding the
REF_PHANDLE, no matter the current size of the cell.  Or always adding
the minimum of 32-bits or the current size of the cell.

Updating the current cell size will also be a trick.  The parser rules
will need to be split up a bit I think so that once the DT_SIZE and
DT_LITERAL are read the current cell size can be updated and then the
celllist can be read.

In any of these solutions I think we still need to track the current
cell size in the data struct.

I don't see any problem with adding LABEL markers to cell lists of
size smaller than 32-bits.

Thoughts?

Thanks,
    Anton

>>> It has the advantage of limiting the number of reserved words created.
>>>  It could also be:
>>>
>>> property = /type/ uint8 <0xab 0xcd>;
>>
>> Not so fond of this one.  The "uint8" would have to be some new
>> lexical type - "identifed" probably, which we'd then have to look up.
>>
>>> Which would allow us to define new types for cell lists without adding
>>> new syntax.  I'm not sure if this is too useful though because the
>>> only types that I can think of that are not summed up by their size
>>> are things like float and double...
>>
>> Yeah, it's my feeling that the < > symtax should remain for integer
>> arrays.  If we need something for other types in future, we should
>> define new, distinct syntax for that when the time comes.
>>
>>> But it does have a nice look to
>>> it in my opinion.  And I would assume that if the /type/ <typename>
>>> was left off it would default to a uint32_t cell list.  So the
>>> additional verbosity of having to indicate it's a different type and
>>> what the type is will only be needed in a few instances.
>>
>> Yes.
>>
>> --
>> David Gibson                    | I'll have my music baroque, and my code
>> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
>>                                | _way_ _around_!
>> http://www.ozlabs.org/~dgibson
>>
>