Re: [PATCH 1/2 v6] annotations: add positions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]




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
>



[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux