On Fri, Sep 14, 2018 at 02:01:09PM +0100, Grant Likely wrote:
> When there is a label inside a sequence of ints at the end of a
> property, an assertion is hit because write_propval() expects all the
> labels to be at the very end of the property data. This is clearly wrong
> behaviour.
>
> To reproduce run: "dtc -O dts tests/label01.dts". dtc fails on property
> /randomnode/blob.
>
> Fix by reworking the write_propval() loop to remove the separate
> iterating over label markers. Instead handle the label markers as part
> of the main marker iteration loop. This guarantees that each label
> marker is handled at the right location, even if all the data markers
> have already been handled, and has the added advantage of making the
> code simpler.
>
> However, a side effect of this code is that a label at the very end of
> an int sequence will be emitted outside the sequence delimiters. For
> example:
>
> Input: intprop = < 1 2 L1: >, L2: < 3 4 L3: > L4:;
> Output: intprop = < 1 2 >, L1: L2: < 3 4 > L3: L4:;
>
> The two representations are equivalent in the data model, but the
> current test case was looking for the former, but needed to be modified
> to look for the later. The alternative would be to render labels before
> closing the sequence, but that makes less sense syntactically because
> labels between sequences are normally to point at the next one, not the
> former. For example:
>
> Input: intprop = < 1 2 L1: >, L2: < 3 4 L3: > L4:;
> Output: intprop = < 1 2 L1: L2: >, < 3 4 L3: L4: >;
>
> DTC doesn't current have the information to know if the label should be
> inside or outside the sequence, but in common usage, it is more likely
> that L1 & L2 refer to the second sequence, not the end of the first.
>
> Fixes: 32b9c6130762 ("Preserve datatype markers when emitting dts
> format")
>
> Reported-by: Łukasz Dobrowolski <lukasz.dobrowolski@xxxxxxxxx>
> Signed-off-by: Grant Likely <grant.likely@xxxxxxx>
> Cc: David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>
> Cc: Rob Herring <robh@xxxxxxxxxx>
Applied, thanks, although..
[snip]
> - fprintf(f, "%s", delim_start[emit_type]);
> + if (m->type == LABEL)
> + fprintf(f, " %s:", m->ref);
>
> - if (chunk_len <= 0)
> + if (emit_type == TYPE_NONE) {
> + assert(chunk_len == 0);
I'm a bit unsure about this assert(). I think you could get a
TYPE_NONE marker with a non-zero chunk length using /incbin/ although
I wasn't able to actually trip it in a brief attempt.
--
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
Attachment:
signature.asc
Description: PGP signature
