On Wed, Sep 21, 2011 at 01:42:10PM -0700, Anton Staaf wrote:
> This function deals with appending literals of various sizes (8, 16
> 32, and 64 bit currently). It handles endianess conversions and
> verifies that the given literal is not larger than the specified
> size.
>
> Signed-off-by: Anton Staaf <robot...@chromium.org>
> Cc: David Gibson <da...@gibson.dropbear.id.au>
> Cc: Jon Loeliger <j...@jdl.com>
> Cc: Grant Likely <grant.lik...@secretlab.ca>
> ---
> data.c | 33 +++++++++++++++++++++++++++++++++
> dtc.h | 1 +
> 2 files changed, 34 insertions(+), 0 deletions(-)
>
> diff --git a/data.c b/data.c
> index b5f3066..37acd6a 100644
> --- a/data.c
> +++ b/data.c
> @@ -175,6 +175,39 @@ struct data data_append_cell(struct data d, cell_t word)
> return data_append_data(d, &beword, sizeof(beword));
> }
>
> +struct data data_append_literal(struct data d, uint64_t value, int len)
I'd prefer to call this data_append_integer() since there are string
and character literals too. Plus by the time we get to the struct
data level, it's not really relevant any more that this came from a
literal in the parser.
And perhaps call it 'bits' or 'size' rather than 'len'. 'len' to me
suggests a byte size rather than a bit size.
> +{
> + uint8_t value_8;
> + uint16_t be_value_16;
> + uint32_t be_value_32;
> + uint64_t be_value_64;
I'd remove the 'be_', it doesn't really add anything of value. Plus
I've mostly avoided explicit references to BE (hence fdtXX_to_cpu() on
the off chance that someone one day is stupid enough to use an LE
variant of the fdt format.
> +
> + if ((len < 64) && (value >= (1ULL << len)))
> + die("Literal value 0x%x too large to fit in %d-bit cell\n",
> + value, len);
This really shouldn't be a die(). In general bad input should not
directly trigger a die() during parse - it should give an error but
continue parse as best it can and only drop out afterwards.
In this case, I think the semantics of an overflow are clear enough
that it shouldn't even be an error per se. Just print a warning, and
mask the number down to the relevant size.
> +
> + switch (len) {
> + case 8:
> + value_8 = value;
> + return data_append_data(d, &value_8, 1);
> +
> + case 16:
> + be_value_16 = cpu_to_fdt16(value);
> + return data_append_data(d, &be_value_16, 2);
> +
> + case 32:
> + be_value_32 = cpu_to_fdt32(value);
> + return data_append_data(d, &be_value_32, 4);
> +
> + case 64:
> + be_value_64 = cpu_to_fdt64(value);
> + return data_append_data(d, &be_value_64, 8);
> +
> + default:
> + die("Invalid literal size (%d)\n", len);
This on the other hand is fine to be a die(), since it's essentially
an assertion that should only be triggered by bad code elsewhere in
dtc itself, not by bad input.
> + }
> +}
> +
> struct data data_append_re(struct data d, const struct fdt_reserve_entry *re)
> {
> struct fdt_reserve_entry bere;
> diff --git a/dtc.h b/dtc.h
> index f37c97e..50433f6 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -109,6 +109,7 @@ struct data data_insert_at_marker(struct data d, struct
> marker *m,
> const void *p, int len);
> struct data data_merge(struct data d1, struct data d2);
> struct data data_append_cell(struct data d, cell_t word);
> +struct data data_append_literal(struct data d, uint64_t word, int len);
> struct data data_append_re(struct data d, const struct fdt_reserve_entry
> *re);
> struct data data_append_addr(struct data d, uint64_t addr);
> struct data data_append_byte(struct data d, uint8_t byte);
--
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
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
