Re: [PATCH v3 12/18] dtc: Add merge_fragment() to merge any fragment

[David Gibson <david@gibson.dropbear.id.au>]

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

On Fri, Nov 06, 2026 at 12:51:39PM -0700, Simon Glass wrote:
> Each grammar action which merges a fragment calls the function for its
> own kind of fragment, sets treesource_error if that fails, and finds out
> whether the file is a plugin using $<flags>-1
> 
> Add merge_fragment() to merge any fragment, choosing the function by the
> type of fragment and whether it is the first. Have each action build a
> struct fragment and pass it to a new merge_parsed_fragment() helper,
> which calls merge_fragment(), sets treesource_error and frees the
> fragment. The functions used by merge_fragment() become static.
> 
> The output does not change.

LGTM.  Again, the logic is a bit more redundant than I'd like, but
it's a reasonable interim step.

> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> (no changes since v2)
> 
> Changes in v2:
> - Use the new constructors and return true on success
> - Use dt_ref instead of fragment_ref
> - Combine the label and path rules earlier, in the path-reference patch
> 
>  dtc-parser.y | 53 ++++++++++++++++++++++++++--------------------------
>  dtc.h        |  5 +----
>  livetree.c   | 42 +++++++++++++++++++++++++++++++++++++----
>  3 files changed, 66 insertions(+), 34 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index abeee0d..9cda518 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -24,6 +24,22 @@ extern void yyerror(char const *s);
>  extern struct dt_info *parser_output;
>  extern bool treesource_error;
>  
> +/*
> + * Merge a fragment into the tree as soon as it is parsed. The flags come
> + * from the actions using $<flags>-1, which relies on the rule being always:
> + *   headers memreserves devicetree
> + * so $-1 is the value of headers
> + */
> +static struct node *merge_parsed_fragment(struct node *dt, struct fragment *f,
> +					  unsigned int flags)
> +{
> +	if (!merge_fragment(&dt, f, flags & DTSF_PLUGIN))
> +		treesource_error = true;
> +	free_fragment(f);
> +
> +	return dt;
> +}
> +
>  %}
>  
>  %union {
> @@ -155,65 +171,50 @@ dt_ref: DT_LABEL_REF | DT_PATH_REF;
>  devicetree:
>  	  '/' nodedef
>  		{
> -			$$ = name_node($2, "");
> +			struct fragment *f = build_fragment_root($2);
> +
> +			$$ = merge_parsed_fragment(NULL, f, $<flags>-1);
>  		}
>  	| devicetree '/' nodedef
>  		{
> -			$$ = merge_nodes($1, $3);
> +			struct fragment *f = build_fragment_root($3);
> +
> +			$$ = merge_parsed_fragment($1, f, $<flags>-1);
>  		}
>  	| dt_ref nodedef
>  		{
>  			struct fragment *f;
>  
>  			f = build_fragment_ref(NULL, NULL, $1, &@1, $2);
> -			/*
> -			 * We rely on the rule being always:
> -			 *   versioninfo plugindecl memreserves devicetree
> -			 * so $-1 is what we want (plugindecl)
> -			 */
> -			if (!merge_first_ref(&$$, f, $<flags>-1 & DTSF_PLUGIN))
> -				treesource_error = true;
> +			$$ = merge_parsed_fragment(NULL, f, $<flags>-1);
>  		}
>  	| devicetree DT_LABEL dt_ref nodedef
>  		{
>  			struct fragment *f;
>  
>  			f = build_fragment_ref($2, &@2, $3, &@3, $4);
> -			if (!merge_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
> -				treesource_error = true;
> -			$$ = $1;
> +			$$ = merge_parsed_fragment($1, f, $<flags>-1);
>  		}
>  	| devicetree dt_ref nodedef
>  		{
>  			struct fragment *f;
>  
>  			f = build_fragment_ref(NULL, NULL, $2, &@2, $3);
> -			/*
> -			 * We rely on the rule being always:
> -			 *   versioninfo plugindecl memreserves devicetree
> -			 * so $-1 is what we want (plugindecl)
> -			 */
> -			if (!merge_ref($1, f, $<flags>-1 & DTSF_PLUGIN))
> -				treesource_error = true;
> -			$$ = $1;
> +			$$ = merge_parsed_fragment($1, f, $<flags>-1);
>  		}
>  	| devicetree DT_DEL_NODE dt_ref ';'
>  		{
>  			struct fragment *f;
>  
>  			f = build_fragment_delete_node($3, &@3);
> -			if (!merge_delete_node($1, f))
> -				treesource_error = true;
> -			$$ = $1;
> +			$$ = merge_parsed_fragment($1, f, $<flags>-1);
>  		}
>  	| devicetree DT_OMIT_NO_REF dt_ref ';'
>  		{
>  			struct fragment *f;
>  
>  			f = build_fragment_omit_if_no_ref($3, &@3);
> -			if (!merge_omit_if_no_ref($1, f))
> -				treesource_error = true;
> -			$$ = $1;
> +			$$ = merge_parsed_fragment($1, f, $<flags>-1);
>  		}
>  	;
>  
> diff --git a/dtc.h b/dtc.h
> index 1befd5c..e223bc2 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -372,10 +372,7 @@ struct fragment *build_fragment_omit_if_no_ref(char *ref,
>  					       struct srcpos *ref_srcpos);
>  struct fragment *append_fragment(struct fragment *list, struct fragment *new);
>  void free_fragment(struct fragment *f);
> -bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin);
> -bool merge_ref(struct node *dt, struct fragment *f, bool plugin);
> -bool merge_delete_node(struct node *dt, struct fragment *f);
> -bool merge_omit_if_no_ref(struct node *dt, struct fragment *f);
> +bool merge_fragment(struct node **dtp, struct fragment *f, bool plugin);
>  
>  struct dt_info {
>  	unsigned int dtsflags;
> diff --git a/livetree.c b/livetree.c
> index 26d39df..a1d5d71 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -580,7 +580,7 @@ static bool is_ref_relative(const char *ref)
>   *
>   * Returns true on success, false on an error, which has been reported.
>   */
> -bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
> +static bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
>  {
>  	bool ok = true;
>  
> @@ -611,7 +611,7 @@ bool merge_first_ref(struct node **dtp, struct fragment *f, bool plugin)
>   *
>   * Returns true on success, false on an error, which has been reported.
>   */
> -bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
> +static bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
>  {
>  	struct node *target;
>  	bool ok = true;
> @@ -651,7 +651,7 @@ bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
>   *
>   * Returns true on success, false on an error, which has been reported.
>   */
> -bool merge_delete_node(struct node *dt, struct fragment *f)
> +static bool merge_delete_node(struct node *dt, struct fragment *f)
>  {
>  	struct node *target = get_node_by_ref(dt, f->ref);
>  
> @@ -672,7 +672,7 @@ bool merge_delete_node(struct node *dt, struct fragment *f)
>   *
>   * Returns true on success, false on an error, which has been reported.
>   */
> -bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
> +static bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
>  {
>  	struct node *target = get_node_by_ref(dt, f->ref);
>  
> @@ -687,6 +687,40 @@ bool merge_omit_if_no_ref(struct node *dt, struct fragment *f)
>  	return true;
>  }
>  
> +/*
> + * Merge a fragment of a dts file into the tree at *dtp, which is NULL for the
> + * first fragment. A reference is looked up in the tree as merged so far.
> + *
> + * Returns true on success, false on an error, which has been reported.
> + */
> +bool merge_fragment(struct node **dtp, struct fragment *f, bool plugin)
> +{
> +	struct node *dt = *dtp;
> +
> +	if (!dt) {
> +		/* The first fragment is the root or, in a plugin, a ref */
> +		if (!f->ref) {
> +			*dtp = name_node(f->node, "");
> +			return true;
> +		}
> +		return merge_first_ref(dtp, f, plugin);
> +	}
> +
> +	switch (f->type) {
> +	case FRAGMENT_NODE:
> +		if (f->ref)
> +			return merge_ref(dt, f, plugin);
> +		*dtp = merge_nodes(dt, f->node);
> +		break;
> +	case FRAGMENT_DELETE_NODE:
> +		return merge_delete_node(dt, f);
> +	case FRAGMENT_OMIT_IF_NO_REF:
> +		return merge_omit_if_no_ref(dt, f);
> +	}
> +
> +	return true;
> +}
> +
>  /*
>   * Tree accessor functions
>   */
> -- 
> 2.43.0
> 
> 

-- 
David Gibson (he or they)	| 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 --]