[PATCH] Fix ambiguous grammar for devicetree rule

[PATCH] Fix ambiguous grammar for devicetree rule

[Grant Likely]

From: Grant Likely <grant-72MMKEujIeWYvcOBcYx38pOp/cRyR9/c930Pai70D+E@public.gmane.org>

Commit 737b2df3, "overlay: Add syntactic sugar version of overlays"
introduced an empty rule for "devicetree" that created ambiguities in
the grammar and causes the following warning:

         BISON dtc-parser.tab.c
  dtc-parser.y: warning: 3 shift/reduce conflicts [-Wconflicts-sr]

Fix the grammar by explicitly testing for the condition the
new overlay grammar wants to use. This means duplicating a very small
amount of grammar processing code, but the alternative seems to be a
more invasive reorganization of the devicetree rule. Better to fix it
this way now and save the reorg for a separate patch

Signed-off-by: Grant Likely <grant.likely-5wv7dgnIgG8@public.gmane.org>
Cc: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
Cc: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
 dtc-parser.y | 17 +++++++++++------
 dtc.h        |  2 +-
 livetree.c   |  3 ++-
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index affc81a..44af170 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -166,7 +166,17 @@ devicetree:
 		{
 			$$ = merge_nodes($1, $3);
 		}
-
+	| DT_REF nodedef
+		{
+			/*
+			 * We rely on the rule being always:
+			 *   versioninfo plugindecl memreserves devicetree
+			 * so $-1 is what we want (plugindecl)
+			 */
+			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);
+		}
 	| devicetree DT_LABEL DT_REF nodedef
 		{
 			struct node *target = get_node_by_ref($1, $3);
@@ -209,11 +219,6 @@ devicetree:
 
 			$$ = $1;
 		}
-	| /* empty */
-		{
-			/* build empty node */
-			$$ = name_node(build_node(NULL, NULL), "");
-		}
 	;
 
 nodedef:
diff --git a/dtc.h b/dtc.h
index 35cf926..84187a1 100644
--- a/dtc.h
+++ b/dtc.h
@@ -203,7 +203,7 @@ struct node *build_node_delete(void);
 struct node *name_node(struct node *node, char *name);
 struct node *chain_node(struct node *first, struct node *list);
 struct node *merge_nodes(struct node *old_node, struct node *new_node);
-void add_orphan_node(struct node *old_node, struct node *new_node, char *ref);
+struct node *add_orphan_node(struct node *old_node, struct node *new_node, char *ref);
 
 void add_property(struct node *node, struct property *prop);
 void delete_property_by_name(struct node *node, char *name);
diff --git a/livetree.c b/livetree.c
index 6846ad2..4a3d893 100644
--- a/livetree.c
+++ b/livetree.c
@@ -216,7 +216,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
 	return old_node;
 }
 
-void add_orphan_node(struct node *dt, struct node *new_node, char *ref)
+struct node * add_orphan_node(struct node *dt, struct node *new_node, char *ref)
 {
 	static unsigned int next_orphan_fragment = 0;
 	struct node *node;
@@ -236,6 +236,7 @@ void add_orphan_node(struct node *dt, struct node *new_node, char *ref)
 	name_node(node, name);
 
 	add_child(dt, node);
+	return dt;
 }
 
 struct node *chain_node(struct node *first, struct node *list)
-- 
2.11.0

Re: [PATCH] Fix ambiguous grammar for devicetree rule

[Rob Herring]

On Mon, Nov 20, 2017 at 11:12 AM, Grant Likely
<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> From: Grant Likely <grant-72MMKEujIeWYvcOBcYx38pOp/cRyR9/c930Pai70D+E@public.gmane.org>

Ummm?

> Commit 737b2df3, "overlay: Add syntactic sugar version of overlays"
> introduced an empty rule for "devicetree" that created ambiguities in
> the grammar and causes the following warning:
>
>          BISON dtc-parser.tab.c
>   dtc-parser.y: warning: 3 shift/reduce conflicts [-Wconflicts-sr]
>
> Fix the grammar by explicitly testing for the condition the
> new overlay grammar wants to use. This means duplicating a very small
> amount of grammar processing code, but the alternative seems to be a
> more invasive reorganization of the devicetree rule. Better to fix it
> this way now and save the reorg for a separate patch

Missing period.

>
> Signed-off-by: Grant Likely <grant.likely-5wv7dgnIgG8@public.gmane.org>
> Cc: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> Cc: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

Re: [PATCH] Fix ambiguous grammar for devicetree rule

[Grant Likely]

On Mon, Nov 20, 2017 at 5:20 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Nov 20, 2017 at 11:12 AM, Grant Likely
> <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>> From: Grant Likely <grant-72MMKEujIeWYvcOBcYx38pOp/cRyR9/c930Pai70D+E@public.gmane.org>
>
> Ummm?

New Linux VM; forgot to do the "git config user.email" step. Bonus
points to there person who figures out my computer naming scheme.

>
>> Commit 737b2df3, "overlay: Add syntactic sugar version of overlays"
>> introduced an empty rule for "devicetree" that created ambiguities in
>> the grammar and causes the following warning:
>>
>>          BISON dtc-parser.tab.c
>>   dtc-parser.y: warning: 3 shift/reduce conflicts [-Wconflicts-sr]
>>
>> Fix the grammar by explicitly testing for the condition the
>> new overlay grammar wants to use. This means duplicating a very small
>> amount of grammar processing code, but the alternative seems to be a
>> more invasive reorganization of the devicetree rule. Better to fix it
>> this way now and save the reorg for a separate patch
>
> Missing period.

thx.

>
>>
>> Signed-off-by: Grant Likely <grant.likely-5wv7dgnIgG8@public.gmane.org>
>> Cc: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>> Cc: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

Re: [PATCH] Fix ambiguous grammar for devicetree rule

[Rob Herring]

On Mon, Nov 20, 2017 at 11:22 AM, Grant Likely
<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> On Mon, Nov 20, 2017 at 5:20 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On Mon, Nov 20, 2017 at 11:12 AM, Grant Likely
>> <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>>> From: Grant Likely <grant-72MMKEujIeWYvcOBcYx38pOp/cRyR9/c930Pai70D+E@public.gmane.org>
>>
>> Ummm?
>
> New Linux VM; forgot to do the "git config user.email" step. Bonus
> points to there person who figures out my computer naming scheme.

It's after your favorite music artists, obviously.

Rob

Re: [PATCH] Fix ambiguous grammar for devicetree rule

[Grant Likely]

On Mon, Nov 20, 2017 at 5:29 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Nov 20, 2017 at 11:22 AM, Grant Likely
> <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>> On Mon, Nov 20, 2017 at 5:20 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>> On Mon, Nov 20, 2017 at 11:12 AM, Grant Likely
>>> <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>>>> From: Grant Likely <grant-72MMKEujIeWYvcOBcYx38pOp/cRyR9/c930Pai70D+E@public.gmane.org>
>>>
>>> Ummm?
>>
>> New Linux VM; forgot to do the "git config user.email" step. Bonus
>> points to there person who figures out my computer naming scheme.
>
> It's after your favorite music artists, obviously.

Oooh, burn. But no. Previous laptop was named Trevor

g.

Re: [PATCH] Fix ambiguous grammar for devicetree rule

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1211 bytes --]

On Mon, Nov 20, 2017 at 11:20:31AM -0600, Rob Herring wrote:
> On Mon, Nov 20, 2017 at 11:12 AM, Grant Likely
> <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> > From: Grant Likely <grant-72MMKEujIeWYvcOBcYx38pOp/cRyR9/c930Pai70D+E@public.gmane.org>
> 
> Ummm?
> 
> > Commit 737b2df3, "overlay: Add syntactic sugar version of overlays"
> > introduced an empty rule for "devicetree" that created ambiguities in
> > the grammar and causes the following warning:
> >
> >          BISON dtc-parser.tab.c
> >   dtc-parser.y: warning: 3 shift/reduce conflicts [-Wconflicts-sr]
> >
> > Fix the grammar by explicitly testing for the condition the
> > new overlay grammar wants to use. This means duplicating a very small
> > amount of grammar processing code, but the alternative seems to be a
> > more invasive reorganization of the devicetree rule. Better to fix it
> > this way now and save the reorg for a separate patch
> 
> Missing period.

I fixed up both of these in my tree.

-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] Fix ambiguous grammar for devicetree rule

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3859 bytes --]

On Mon, Nov 20, 2017 at 05:12:18PM +0000, Grant Likely wrote:
> From: Grant Likely <grant-72MMKEujIeWYvcOBcYx38pOp/cRyR9/c930Pai70D+E@public.gmane.org>
> 
> Commit 737b2df3, "overlay: Add syntactic sugar version of overlays"
> introduced an empty rule for "devicetree" that created ambiguities in
> the grammar and causes the following warning:
> 
>          BISON dtc-parser.tab.c
>   dtc-parser.y: warning: 3 shift/reduce conflicts [-Wconflicts-sr]
> 
> Fix the grammar by explicitly testing for the condition the
> new overlay grammar wants to use. This means duplicating a very small
> amount of grammar processing code, but the alternative seems to be a
> more invasive reorganization of the devicetree rule. Better to fix it
> this way now and save the reorg for a separate patch
> 
> Signed-off-by: Grant Likely <grant.likely-5wv7dgnIgG8@public.gmane.org>
> Cc: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> Cc: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

Strictly speaking this makes a behavioural change, since an entirely
empty overlay is no longer allowed.  I think that's an acceptable
change, though.

Applied.

> ---
>  dtc-parser.y | 17 +++++++++++------
>  dtc.h        |  2 +-
>  livetree.c   |  3 ++-
>  3 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index affc81a..44af170 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -166,7 +166,17 @@ devicetree:
>  		{
>  			$$ = merge_nodes($1, $3);
>  		}
> -
> +	| DT_REF nodedef
> +		{
> +			/*
> +			 * We rely on the rule being always:
> +			 *   versioninfo plugindecl memreserves devicetree
> +			 * so $-1 is what we want (plugindecl)
> +			 */
> +			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);
> +		}
>  	| devicetree DT_LABEL DT_REF nodedef
>  		{
>  			struct node *target = get_node_by_ref($1, $3);
> @@ -209,11 +219,6 @@ devicetree:
>  
>  			$$ = $1;
>  		}
> -	| /* empty */
> -		{
> -			/* build empty node */
> -			$$ = name_node(build_node(NULL, NULL), "");
> -		}
>  	;
>  
>  nodedef:
> diff --git a/dtc.h b/dtc.h
> index 35cf926..84187a1 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -203,7 +203,7 @@ struct node *build_node_delete(void);
>  struct node *name_node(struct node *node, char *name);
>  struct node *chain_node(struct node *first, struct node *list);
>  struct node *merge_nodes(struct node *old_node, struct node *new_node);
> -void add_orphan_node(struct node *old_node, struct node *new_node, char *ref);
> +struct node *add_orphan_node(struct node *old_node, struct node *new_node, char *ref);
>  
>  void add_property(struct node *node, struct property *prop);
>  void delete_property_by_name(struct node *node, char *name);
> diff --git a/livetree.c b/livetree.c
> index 6846ad2..4a3d893 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -216,7 +216,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>  	return old_node;
>  }
>  
> -void add_orphan_node(struct node *dt, struct node *new_node, char *ref)
> +struct node * add_orphan_node(struct node *dt, struct node *new_node, char *ref)
>  {
>  	static unsigned int next_orphan_fragment = 0;
>  	struct node *node;
> @@ -236,6 +236,7 @@ void add_orphan_node(struct node *dt, struct node *new_node, char *ref)
>  	name_node(node, name);
>  
>  	add_child(dt, node);
> +	return dt;
>  }
>  
>  struct node *chain_node(struct node *first, struct node *list)

-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]