On Tue, 13 Nov 2018, David Gibson wrote:
> On Fri, Oct 26, 2018 at 05:44:33PM +0200, Julia Lawall wrote:
> > Extend the parser to record positions, in build_node, build_node_delete,
> > and build_property.
> >
> > srcpos structures are added to the property and node types, and to the
> > parameter lists of the above functions that construct these types.
> > Nodes and properties that are created by the compiler rather than from
> > parsing source code have NULL as the srcpos value.
> >
> > merge_nodes, defined in livetree.c, uses srcpos_extend to combine
> > multiple positions, resulting in a list of positions. srcpos_extend is
> > defined in srcpos.c. New elements are added at the end. The srcpos
> > type, define in srcpos.h, is now a list structure with a next field.
> >
> > Another change to srcpos.c is to make srcpos_copy always do a full copy,
> > including a copy of the file substructure. This is required because
> > when dtc is used on the output of cpp, the successive detected file
> > names overwrite the file name in the file structure. The next field
> > does not need to be deep copied, because it is only updated in newly
> > copied positions and the positions to which it points have also been
> > copied. File names are only updated in uncopied position structures.
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@xxxxxxx>
> > ---
> >
> > v6: SoB added, v5 information added
> >
> > v5: The main change is that srcpos_extend now concatenates the arguments in
> > the opposite order (new information added after the existing information),
> > which makes it possible to remove all of the complexity of the previous
> > implementation that was designed to avoid introducing circular
> > lists.
>
> Sorry I've taken a long time to look at this; I've been travelling
> then dealing with a backlog of work. I've now applied this.
No problem. Thanks. I'll take a look at the leak soon.
julia
>
> >
> > dtc-parser.y | 13 ++++++++-----
> > dtc.h | 10 +++++++---
> > flattree.c | 4 ++--
> > fstree.c | 5 +++--
> > livetree.c | 33 ++++++++++++++++++++++-----------
> > srcpos.c | 22 ++++++++++++++++++++++
> > srcpos.h | 3 +++
> > 7 files changed, 67 insertions(+), 23 deletions(-)
> >
> > diff --git a/dtc-parser.y b/dtc-parser.y
> > index deda663..2ec981e 100644
> > --- a/dtc-parser.y
> > +++ b/dtc-parser.y
> > @@ -180,7 +180,10 @@ devicetree:
> > */
> > if (!($<flags>-1 & DTSF_PLUGIN))
> > ERROR(&@2, "Label or path %s not found", $1);
> > - $$ = add_orphan_node(name_node(build_node(NULL, NULL), ""), $2, $1);
> > + $$ = add_orphan_node(
> > + name_node(build_node(NULL, NULL, NULL),
> > + ""),
> > + $2, $1);
> > }
> > | devicetree DT_LABEL dt_ref nodedef
> > {
> > @@ -260,7 +263,7 @@ devicetree:
> > nodedef:
> > '{' proplist subnodes '}' ';'
> > {
> > - $$ = build_node($2, $3);
> > + $$ = build_node($2, $3, &@$);
> > }
> > ;
> >
> > @@ -278,11 +281,11 @@ proplist:
> > propdef:
> > DT_PROPNODENAME '=' propdata ';'
> > {
> > - $$ = build_property($1, $3);
> > + $$ = build_property($1, $3, &@$);
> > }
> > | DT_PROPNODENAME ';'
> > {
> > - $$ = build_property($1, empty_data);
> > + $$ = build_property($1, empty_data, &@$);
> > }
> > | DT_DEL_PROP DT_PROPNODENAME ';'
> > {
> > @@ -563,7 +566,7 @@ subnode:
> > }
> > | DT_DEL_NODE DT_PROPNODENAME ';'
> > {
> > - $$ = name_node(build_node_delete(), $2);
> > + $$ = name_node(build_node_delete(&@$), $2);
> > }
> > | DT_OMIT_NO_REF subnode
> > {
> > diff --git a/dtc.h b/dtc.h
> > index cbe5415..8722cbc 100644
> > --- a/dtc.h
> > +++ b/dtc.h
> > @@ -158,6 +158,7 @@ struct property {
> > struct property *next;
> >
> > struct label *labels;
> > + struct srcpos *srcpos;
> > };
> >
> > struct node {
> > @@ -177,6 +178,7 @@ struct node {
> >
> > struct label *labels;
> > const struct bus_type *bus;
> > + struct srcpos *srcpos;
> >
> > bool omit_if_unused, is_referenced;
> > };
> > @@ -205,13 +207,15 @@ struct node {
> > void add_label(struct label **labels, char *label);
> > void delete_labels(struct label **labels);
> >
> > -struct property *build_property(char *name, struct data val);
> > +struct property *build_property(char *name, struct data val,
> > + struct srcpos *srcpos);
> > struct property *build_property_delete(char *name);
> > struct property *chain_property(struct property *first, struct property *list);
> > struct property *reverse_properties(struct property *first);
> >
> > -struct node *build_node(struct property *proplist, struct node *children);
> > -struct node *build_node_delete(void);
> > +struct node *build_node(struct property *proplist, struct node *children,
> > + struct srcpos *srcpos);
> > +struct node *build_node_delete(struct srcpos *srcpos);
> > struct node *name_node(struct node *node, char *name);
> > struct node *omit_node_if_unused(struct node *node);
> > struct node *reference_node(struct node *node);
> > diff --git a/flattree.c b/flattree.c
> > index 851ea87..acf04c3 100644
> > --- a/flattree.c
> > +++ b/flattree.c
> > @@ -692,7 +692,7 @@ static struct property *flat_read_property(struct inbuf *dtbuf,
> >
> > val = flat_read_data(dtbuf, proplen);
> >
> > - return build_property(name, val);
> > + return build_property(name, val, NULL);
> > }
> >
> >
> > @@ -750,7 +750,7 @@ static struct node *unflatten_tree(struct inbuf *dtbuf,
> > char *flatname;
> > uint32_t val;
> >
> > - node = build_node(NULL, NULL);
> > + node = build_node(NULL, NULL, NULL);
> >
> > flatname = flat_read_string(dtbuf);
> >
> > diff --git a/fstree.c b/fstree.c
> > index ae7d06c..1e7eeba 100644
> > --- a/fstree.c
> > +++ b/fstree.c
> > @@ -34,7 +34,7 @@ static struct node *read_fstree(const char *dirname)
> > if (!d)
> > die("Couldn't opendir() \"%s\": %s\n", dirname, strerror(errno));
> >
> > - tree = build_node(NULL, NULL);
> > + tree = build_node(NULL, NULL, NULL);
> >
> > while ((de = readdir(d)) != NULL) {
> > char *tmpname;
> > @@ -60,7 +60,8 @@ static struct node *read_fstree(const char *dirname)
> > } else {
> > prop = build_property(xstrdup(de->d_name),
> > data_copy_file(pfile,
> > - st.st_size));
> > + st.st_size),
> > + NULL);
> > add_property(tree, prop);
> > fclose(pfile);
> > }
> > diff --git a/livetree.c b/livetree.c
> > index 4ff0679..7a2e644 100644
> > --- a/livetree.c
> > +++ b/livetree.c
> > @@ -19,6 +19,7 @@
> > */
> >
> > #include "dtc.h"
> > +#include "srcpos.h"
> >
> > /*
> > * Tree building functions
> > @@ -50,7 +51,8 @@ void delete_labels(struct label **labels)
> > label->deleted = 1;
> > }
> >
> > -struct property *build_property(char *name, struct data val)
> > +struct property *build_property(char *name, struct data val,
> > + struct srcpos *srcpos)
> > {
> > struct property *new = xmalloc(sizeof(*new));
> >
> > @@ -58,6 +60,7 @@ struct property *build_property(char *name, struct data val)
> >
> > new->name = name;
> > new->val = val;
> > + new->srcpos = srcpos_copy(srcpos);
> >
> > return new;
> > }
> > @@ -97,7 +100,8 @@ struct property *reverse_properties(struct property *first)
> > return head;
> > }
> >
> > -struct node *build_node(struct property *proplist, struct node *children)
> > +struct node *build_node(struct property *proplist, struct node *children,
> > + struct srcpos *srcpos)
> > {
> > struct node *new = xmalloc(sizeof(*new));
> > struct node *child;
> > @@ -106,6 +110,7 @@ struct node *build_node(struct property *proplist, struct node *children)
> >
> > new->proplist = reverse_properties(proplist);
> > new->children = children;
> > + new->srcpos = srcpos_copy(srcpos);
> >
> > for_each_child(new, child) {
> > child->parent = new;
> > @@ -114,13 +119,14 @@ struct node *build_node(struct property *proplist, struct node *children)
> > return new;
> > }
> >
> > -struct node *build_node_delete(void)
> > +struct node *build_node_delete(struct srcpos *srcpos)
> > {
> > struct node *new = xmalloc(sizeof(*new));
> >
> > memset(new, 0, sizeof(*new));
> >
> > new->deleted = 1;
> > + new->srcpos = srcpos_copy(srcpos);
> >
> > return new;
> > }
> > @@ -183,6 +189,8 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
> >
> > old_prop->val = new_prop->val;
> > old_prop->deleted = 0;
> > + free(old_prop->srcpos);
> > + old_prop->srcpos = new_prop->srcpos;
> > free(new_prop);
> > new_prop = NULL;
> > break;
> > @@ -223,6 +231,8 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
> > add_child(old_node, new_child);
> > }
> >
> > + old_node->srcpos = srcpos_extend(old_node->srcpos, new_node->srcpos);
> > +
> > /* The new node contents are now merged into the old node. Free
> > * the new node. */
> > free(new_node);
> > @@ -241,18 +251,18 @@ struct node * add_orphan_node(struct node *dt, struct node *new_node, char *ref)
> > if (ref[0] == '/') {
> > d = data_append_data(d, ref, strlen(ref) + 1);
> >
> > - p = build_property("target-path", d);
> > + p = build_property("target-path", d, NULL);
> > } else {
> > d = data_add_marker(d, REF_PHANDLE, ref);
> > d = data_append_integer(d, 0xffffffff, 32);
> >
> > - p = build_property("target", d);
> > + p = build_property("target", d, NULL);
> > }
> >
> > xasprintf(&name, "fragment@%u",
> > next_orphan_fragment++);
> > name_node(new_node, "__overlay__");
> > - node = build_node(p, new_node);
> > + node = build_node(p, new_node, NULL);
> > name_node(node, name);
> >
> > add_child(dt, node);
> > @@ -351,7 +361,7 @@ void append_to_property(struct node *node,
> > p->val = d;
> > } else {
> > d = data_append_data(empty_data, data, len);
> > - p = build_property(name, d);
> > + p = build_property(name, d, NULL);
> > add_property(node, p);
> > }
> > }
> > @@ -609,11 +619,11 @@ cell_t get_node_phandle(struct node *root, struct node *node)
> >
> > if (!get_property(node, "linux,phandle")
> > && (phandle_format & PHANDLE_LEGACY))
> > - add_property(node, build_property("linux,phandle", d));
> > + add_property(node, build_property("linux,phandle", d, NULL));
> >
> > if (!get_property(node, "phandle")
> > && (phandle_format & PHANDLE_EPAPR))
> > - add_property(node, build_property("phandle", d));
> > + add_property(node, build_property("phandle", d, NULL));
> >
> > /* If the node *does* have a phandle property, we must
> > * be dealing with a self-referencing phandle, which will be
> > @@ -787,7 +797,7 @@ static struct node *build_and_name_child_node(struct node *parent, char *name)
> > {
> > struct node *node;
> >
> > - node = build_node(NULL, NULL);
> > + node = build_node(NULL, NULL, NULL);
> > name_node(node, xstrdup(name));
> > add_child(parent, node);
> >
> > @@ -849,7 +859,8 @@ static void generate_label_tree_internal(struct dt_info *dti,
> > /* insert it */
> > p = build_property(l->label,
> > data_copy_mem(node->fullpath,
> > - strlen(node->fullpath) + 1));
> > + strlen(node->fullpath) + 1),
> > + NULL);
> > add_property(an, p);
> > }
> >
> > diff --git a/srcpos.c b/srcpos.c
> > index cb6ed0e..cba1c0f 100644
> > --- a/srcpos.c
> > +++ b/srcpos.c
> > @@ -207,6 +207,7 @@ struct srcpos srcpos_empty = {
> > .last_line = 0,
> > .last_column = 0,
> > .file = NULL,
> > + .next = NULL,
> > };
> >
> > void srcpos_update(struct srcpos *pos, const char *text, int len)
> > @@ -234,13 +235,34 @@ struct srcpos *
> > srcpos_copy(struct srcpos *pos)
> > {
> > struct srcpos *pos_new;
> > + struct srcfile_state *srcfile_state;
> > +
> > + if (!pos)
> > + return NULL;
> >
> > pos_new = xmalloc(sizeof(struct srcpos));
> > memcpy(pos_new, pos, sizeof(struct srcpos));
> >
> > + /* allocate without free */
> > + srcfile_state = xmalloc(sizeof(struct srcfile_state));
> > + memcpy(srcfile_state, pos->file, sizeof(struct srcfile_state));
> > + pos_new->file = srcfile_state;
> > +
> > return pos_new;
> > }
> >
> > +struct srcpos *srcpos_extend(struct srcpos *pos, struct srcpos *newtail)
> > +{
> > + struct srcpos *p;
> > +
> > + if (!pos)
> > + return newtail;
> > +
> > + for (p = pos; p->next != NULL; p = p->next);
> > + p->next = newtail;
> > + return pos;
> > +}
> > +
> > char *
> > srcpos_string(struct srcpos *pos)
> > {
> > diff --git a/srcpos.h b/srcpos.h
> > index 9ded12a..d88e7cb 100644
> > --- a/srcpos.h
> > +++ b/srcpos.h
> > @@ -74,6 +74,7 @@ struct srcpos {
> > int last_line;
> > int last_column;
> > struct srcfile_state *file;
> > + struct srcpos *next;
> > };
> >
> > #define YYLTYPE struct srcpos
> > @@ -105,6 +106,8 @@ extern struct srcpos srcpos_empty;
> >
> > extern void srcpos_update(struct srcpos *pos, const char *text, int len);
> > extern struct srcpos *srcpos_copy(struct srcpos *pos);
> > +extern struct srcpos *srcpos_extend(struct srcpos *new_srcpos,
> > + struct srcpos *old_srcpos);
> > extern char *srcpos_string(struct srcpos *pos);
> >
> > extern void PRINTF(3, 0) srcpos_verror(struct srcpos *pos, const char *prefix,
>
> --
> 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
>
