Re: [PATCH v4 2/3] dtc: Support character literals in cell lists

On Mon, Sep 19, 2011 at 07:54:09PM -0700, Anton Staaf wrote:

On Mon, Sep 19, 2011 at 5:59 PM, David Gibson
[off-list ref] wrote:

quoted

On Mon, Sep 19, 2011 at 09:45:34AM -0700, Anton Staaf wrote:

quoted

On Sun, Sep 18, 2011 at 7:00 PM, David Gibson
[off-list ref] wrote:

quoted

On Sat, Sep 17, 2011 at 11:49:21AM -0500, Jon Loeliger wrote:

quoted

quoted

On Fri, Sep 09, 2011 at 12:16:30PM -0700, Anton Staaf wrote:

quoted

With this patch the following property assignment:

    property = <0x12345678 'a' '\r' 100>;

is equivalent to:

    property = <0x12345678 0x00000061 0x0000000D 0x00000064>

Signed-off-by: Anton Staaf <redacted>

Acked-by: David Gibson <redacted>

So, I *think* we want to wait until the question of size
is resolved some more, right?  Or, take this in any event
as "without a type indicator they are all 32-bit values"?

No this patch is fine to take without changing the cell size
semantics.  It's just that it becomes a lot more useful when we do get
those.

Yup, I'm working on a size patch by the way.  Any comments on my
previous post about it would be helpful.  But in the mean time I'm
going ahead with a solution where the current cell size is stored in
the "struct data" type references are not allowed in cell lists of
size other than 32 bits.

Ah, sorry, I meant to give comments on that earlier but got
sidetracked.

Storing the cell size in struct data doesn't really work - a single
property could be assembled from several cell lists of different
sizes.  By the time the reference substitution happens, they will have
been all merged into a single struct data.

I think prohibiting cell references anywhere but 32-bit cell lists is
the right approach, but we need to work out a way to do the check
during the parse phase.

Yes absolutely.  My intent with storing the current cell size in the
data struct was to use that to do the parse time rejection of
references in all but 32-bit cell lists.  That is the current cell
size would not be used past parsing, for exactly the reason you
mention.  My thought was that every creation of a cell list would set
the current cell size to 32 or the value defined by /size/ and the
closing '>' would set it back to 0.  The empty_data initializer would
set the current cell size to 0, even though it should never be used
outside of the <...> context.  Then when a reference or literal are to
be appended to the current data struct we can check for the value 32
if it's a reference, and otherwise use the current cell size to
validate (ensure it fits in the current cell size) and pad (add
leading zero's) the literal.  Padding won't actually be required if
the literal parsing routine always returns a 64-bit value.

Oh, I see.  I guess that would work, but it's a really nasty misuse of
a long-term data structure to store some short-term information.  No,
I'm pretty sure we can rearrange the grammer to handle this more
cleanly.

Maybe something like this:

cellarrayprefix = DT_SIZE DT_LITERAL '<'
                | cellarrayprefix cellval
		| cellarrayprefix DT_REF
		| cellarrayprefix DT_LABEL

The semantic type of cellarray prefix would need to contain both a
struct data and a int for the cell size, which is a bit fiddly, but
should be doable.  The DT_REF case can check the cell size part of $1
and error if it's no 32-bit.

-- 
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