On Wed, Oct 10, 2018 at 09:15:04AM -0500, Rob Herring wrote:
> Commit 32b9c6130762 ("Preserve datatype markers when emitting dts format")
> fails to split string lists into multiple strings and instead just outputs
> a "\0" between strings. This is broken when the input is dtb or fs tree.
Sorry I've take forever to get around to replying to this.
> This could be fixed in the dts output where the problem was introduced,
> but fix it by adding markers on the input side instead. This will allow
> for any possible future uses of type markers.
I actually don't like this approach and would prefer to handle it
purely in the output. At present the output has two clear cases: a)
it has TYPE markers and so knows the right way to present things, or
b) it doesn't, and has to guess.
If we add type markers like this we're putting another place where
we're guessing about the structure - I'd prefer to keep all the
guessing to one spot. And likewise I'd like the presence of a TYPE
marker to unambiguously indicate that we have solid information
indicating how we should format things, rather than possibly being a
guess.
> Reported-by: Stewart Smith <stewart@xxxxxxxxxxxxx>
> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
> ---
> treesource.c | 63 +++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 50 insertions(+), 13 deletions(-)
>
> diff --git a/treesource.c b/treesource.c
> index 93fd8ac1c513..09f0512060a9 100644
> --- a/treesource.c
> +++ b/treesource.c
> @@ -175,7 +175,43 @@ static const char *delim_end[] = {
> [TYPE_STRING] = "",
> };
>
> -static enum markertype guess_value_type(struct property *prop)
> +
> +static struct marker *add_int_marker(struct property *prop, enum markertype type)
> +{
> + struct marker *m;
> +
> + m = xmalloc(sizeof(*m));
> + m->offset = 0;
> + m->type = type;
> + m->ref = NULL;
> + m->next = NULL;
> +
> + return m;
> +}
> +
> +static struct marker *add_string_markers(struct property *prop)
> +{
> + int i;
> + int len = prop->val.len;
> + const char *p = prop->val.val;
> + struct marker *m = NULL, *prev_m = NULL;
> +
> + for (i = len - 1; i >= 0; i--) {
> + if (i && !(p[i] && p[i - 1] == '\0'))
> + continue;
> +
> + m = xmalloc(sizeof(*m));
> + m->offset = i;
> + m->type = TYPE_STRING;
> + m->ref = NULL;
> + m->next = prev_m;
> +
> + prev_m = m;
> + }
> + return m;
> +}
> +
> +static struct marker *gen_missing_markers(struct property *prop)
> {
> int len = prop->val.len;
> const char *p = prop->val.val;
> @@ -200,20 +236,20 @@ static enum markertype guess_value_type(struct property *prop)
>
> if ((p[len-1] == '\0') && (nnotstring == 0) && (nnul < (len-nnul))
> && (nnotstringlbl == 0)) {
> - return TYPE_STRING;
> + return add_string_markers(prop);
> } else if (((len % sizeof(cell_t)) == 0) && (nnotcelllbl == 0)) {
> - return TYPE_UINT32;
> + return add_int_marker(prop, TYPE_UINT32);
> }
>
> - return TYPE_UINT8;
> + return add_int_marker(prop, TYPE_UINT8);
> }
>
> static void write_propval(FILE *f, struct property *prop)
> {
> size_t len = prop->val.len;
> struct marker *m = prop->val.markers;
> - struct marker dummy_marker;
> enum markertype emit_type = TYPE_NONE;
> + struct marker *dummy_markers = NULL;
>
> if (len == 0) {
> fprintf(f, ";\n");
> @@ -222,14 +258,9 @@ static void write_propval(FILE *f, struct property *prop)
>
> fprintf(f, " =");
>
> - if (!next_type_marker(m)) {
> - /* data type information missing, need to guess */
> - dummy_marker.type = guess_value_type(prop);
> - dummy_marker.next = prop->val.markers;
> - dummy_marker.offset = 0;
> - dummy_marker.ref = NULL;
> - m = &dummy_marker;
> - }
> + /* If data type information is missing, need to guess */
> + if (!next_type_marker(m))
> + m = dummy_markers = gen_missing_markers(prop);
>
> for_each_marker(m) {
> size_t chunk_len = (m->next ? m->next->offset : len) - m->offset;
> @@ -274,6 +305,12 @@ static void write_propval(FILE *f, struct property *prop)
> }
> }
> fprintf(f, ";\n");
> +
> + while (dummy_markers) {
> + m = dummy_markers;
> + dummy_markers = dummy_markers->next;
> + free(m);
> + }
> }
>
> static void write_tree_source_node(FILE *f, struct node *tree, int level)
--
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
