Re: [PATCH 8/9 V3] Add documentation for the new DTS language.

[Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>]

On Tue, Mar 2, 2010 at 4:12 PM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Mon, Mar 01, 2010 at 02:17:47PM -0800, Stephen Neuendorffer wrote:
>> > -----Original Message-----
>> > From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On Behalf Of Grant Likely
>> > Sent: Monday, March 01, 2010 1:49 PM
>> > To: Stephen Neuendorffer
>> > Cc: Yoder Stuart-B08248; Wood Scott-B07421; devicetree-discuss@ozlabs.org; John Williams; Jeremy Kerr
>> > Subject: Re: [PATCH 8/9 V3] Add documentation for the new DTS language.
> [snip]
>> > This is never legal.  If deleting by label, it must be at the top
>> > level.  It doesn't make sense to use a label reference inside a node
>> > block, since the node block is already supposed to define where you
>> > are working in the tree.
>> >
>> > >        delete(bar); // May or may not be Illegal, since don't know what bar-label references
>> >
>> > No longer an issue since the previous line is illegal.
>> >
>> > Also, must be either delete-node() or delete-prop() since nodes and
>> > properties can use the same names.
>>
>> So, is it true that a tree which is overlayed on another tree can be
>> independently verified to be independent of internal ordering?  This
>> would be nice if so.
>
> Hrm, yes and no.  Most of our tests for duplicate labels and so forth
> are only performed after all the overlay/merging is completed.
>
> Which brings up another inconsistency in the current processing.
> Although:
>
>        / {
>                foo = "bar";
>                foo = "baz";
>        };
>
> is illegal and will cause an error, the way that merge_nodes() is
> implemented means that the following will be accepted:
>
>        / {
>        };
>        / {
>                foo = "bar";
>                foo = "baz";
>        };
>
> (and the foo property will have the final value "baz").

That should be easy to fix.  How about the following change to prevent
duplicate properties even getting added to the livetree.  This patch
isn't complete, it breaks some of the test cases because it fails in a
different way, and it makes some of the post-checks redundant.

g.

commit f676848929e6764b3a6c14f14eafacdfcaab5f15
Author: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Date:   Wed Mar 3 09:15:30 2010 -0700

    Don't add duplicate properties to the live tree

diff --git a/dtc-parser.y b/dtc-parser.y
index ed87d3b..053ae67 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -156,6 +156,11 @@ proplist:
 		}
 	| proplist propdef
 		{
+			if (find_property($1, $2->name)) {
+				yyerror("syntax error: duplicate property");
+				YYERROR;
+			}
+
 			$$ = chain_property($2, $1);
 		}
 	;
diff --git a/dtc.h b/dtc.h
index b36ac5d..78033fb 100644
--- a/dtc.h
+++ b/dtc.h
@@ -170,6 +170,7 @@ void add_label(struct label **labels, char *label);
 struct property *build_property(char *name, struct data val);
 struct property *chain_property(struct property *first, struct property *list);
 struct property *reverse_properties(struct property *first);
+struct property *find_property(struct property *proplist, const char *name);

 struct node *build_node(struct property *proplist, struct node *children);
 struct node *name_node(struct node *node, char *name);
diff --git a/livetree.c b/livetree.c
index 13c5f10..924e131 100644
--- a/livetree.c
+++ b/livetree.c
@@ -74,6 +74,15 @@ struct property *reverse_properties(struct property *first)
 	return head;
 }

+struct property *find_property(struct property *proplist, const char *name)
+{
+	for (; proplist; proplist = proplist->next)
+		if (streq(proplist->name, name))
+			return proplist;
+
+	return NULL;
+}
+
 struct node *build_node(struct property *proplist, struct node *children)
 {
 	struct node *new = xmalloc(sizeof(*new));


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.