> On 14 Sep 2018, at 19:39, Rob Herring <robh@xxxxxxxxxx> 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.
>
> 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’m not keen putting the resolution in the input stage. It creates a weird split between when the type predictions happen. Strings are predicted at input, and everything else at output. I would be happier if a middle processing stage were added to go over the entire tree and add missing data tokens. Then the output could drop all data prediction logic.
g.
>
> Reported-by: Stewart Smith <stewart@xxxxxxxxxxxxx>
> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
> ---
> We can probably also remove the string type guessing code from the
> dts output as with this I don't think there's any way to not have
> TYPE_STRING markers anymore.
>
> data.c | 47 ++++++++++++++++++++++++++++++++++++++++++++---
> dtc.h | 1 +
> flattree.c | 1 +
> fstree.c | 6 +++---
> 4 files changed, 49 insertions(+), 6 deletions(-)
>
> diff --git a/data.c b/data.c
> index 4a204145cc7b..086cce311c65 100644
> --- a/data.c
> +++ b/data.c
> @@ -95,7 +95,6 @@ struct data data_copy_file(FILE *f, size_t maxlen)
> {
> struct data d = empty_data;
>
> - d = data_add_marker(d, TYPE_NONE, NULL);
> while (!feof(f) && (d.len < maxlen)) {
> size_t chunksize, ret;
>
> @@ -239,12 +238,12 @@ struct data data_append_align(struct data d, int align)
> return data_append_zeroes(d, newlen - d.len);
> }
>
> -struct data data_add_marker(struct data d, enum markertype type, char *ref)
> +static struct data data_add_marker_offset(struct data d, enum markertype type, char *ref, int offset)
> {
> struct marker *m;
>
> m = xmalloc(sizeof(*m));
> - m->offset = d.len;
> + m->offset = offset;
> m->type = type;
> m->ref = ref;
> m->next = NULL;
> @@ -252,6 +251,48 @@ struct data data_add_marker(struct data d, enum markertype type, char *ref)
> return data_append_markers(d, m);
> }
>
> +struct data data_add_marker(struct data d, enum markertype type, char *ref)
> +{
> + return data_add_marker_offset(d, type, ref, d.len);
> +}
> +
> +static bool isstring(char c)
> +{
> + return (isprint((unsigned char)c)
> + || (c == '\0')
> + || strchr("\a\b\t\n\v\f\r", c));
> +}
> +
> +
> +struct data data_add_type_markers(struct data d)
> +{
> + int len = d.len;
> + const char *p = d.val;
> + int nnul = 0;
> + int i;
> +
> + if (d.markers || !len)
> + return d;
> +
> + for (i = 0; i < len; i++) {
> + if (! isstring(p[i]))
> + return data_add_marker(d, TYPE_NONE, NULL);
> + if (p[i] == '\0')
> + nnul++;
> + }
> +
> + if (!((p[len-1] == '\0') && (nnul < (len-nnul))))
> + return data_add_marker(d, TYPE_NONE, NULL);
> +
> + for (i = 0; i < len; i++) {
> + if (!p[i] || !(i == 0 || p[i - 1] == '\0'))
> + continue;
> +
> + d = data_add_marker_offset(d, TYPE_STRING, NULL, i);
> + }
> + return d;
> +}
> +
> bool data_is_one_string(struct data d)
> {
> int i;
> diff --git a/dtc.h b/dtc.h
> index cbe541525c2c..c049ec3bc534 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -131,6 +131,7 @@ struct data data_append_zeroes(struct data d, int len);
> struct data data_append_align(struct data d, int align);
>
> struct data data_add_marker(struct data d, enum markertype type, char *ref);
> +struct data data_add_type_markers(struct data d);
>
> bool data_is_one_string(struct data d);
>
> diff --git a/flattree.c b/flattree.c
> index 851ea87dbc0f..28b3f36cf1d5 100644
> --- a/flattree.c
> +++ b/flattree.c
> @@ -691,6 +691,7 @@ static struct property *flat_read_property(struct inbuf *dtbuf,
> flat_realign(dtbuf, 8);
>
> val = flat_read_data(dtbuf, proplen);
> + val = data_add_type_markers(val);
>
> return build_property(name, val);
> }
> diff --git a/fstree.c b/fstree.c
> index ae7d06c3c492..451d3e6e2896 100644
> --- a/fstree.c
> +++ b/fstree.c
> @@ -58,9 +58,9 @@ static struct node *read_fstree(const char *dirname)
> "WARNING: Cannot open %s: %s\n",
> tmpname, strerror(errno));
> } else {
> - prop = build_property(xstrdup(de->d_name),
> - data_copy_file(pfile,
> - st.st_size));
> + struct data data = data_add_type_markers(
> + data_copy_file(pfile, st.st_size));
> + prop = build_property(xstrdup(de->d_name), data);
> add_property(tree, prop);
> fclose(pfile);
> }
> --
> 2.17.1
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
