Re: [PATCH v3 07/18] dtc: Move label-reference handling out of the grammar

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

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

On Fri, Nov 06, 2026 at 12:51:34PM -0700, Simon Glass wrote:
> The grammar action for a top-level &label { ... } holds its merge logic,
> including making an overlay fragment in a plugin when the label is not
> found.
> 
> Move it into a new merge_ref(), passing it a struct fragment for the
> reference and node.
> 
> The output does not change.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> 
> (no changes since v2)
> 
> Changes in v2:
> - Rename merge_label_ref() to merge_ref()
> - Build the fragment in one step, with build_fragment_ref()
> - Return true on success from merge_ref()
> - Check the fragment type with assert()
> 
>  dtc-parser.y | 23 +++++++++--------------
>  dtc.h        |  1 +
>  livetree.c   | 26 ++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 14 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 9b9b6db..c58a66d 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -214,21 +214,16 @@ devicetree:
>  		}
>  	| devicetree DT_LABEL_REF nodedef
>  		{
> -			struct node *target = get_node_by_ref($1, $2);
> +			struct fragment *f;
>  
> -			if (target) {
> -				merge_nodes(target, $3);
> -			} else {
> -				/*
> -				 * We rely on the rule being always:
> -				 *   versioninfo plugindecl memreserves devicetree
> -				 * so $-1 is what we want (plugindecl)
> -				 */
> -				if ($<flags>-1 & DTSF_PLUGIN)
> -					add_orphan_node($1, $3, $2);
> -				else
> -					ERROR(&@2, "Label or path %s not found", $2);
> -			}
> +			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;
>  		}
>  	| devicetree DT_DEL_NODE dt_ref ';'
> diff --git a/dtc.h b/dtc.h
> index 8c4d7bf..9b4389b 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -373,6 +373,7 @@ struct fragment *build_fragment_omit_if_no_ref(char *ref,
>  struct fragment *append_fragment(struct fragment *list, struct fragment *new);
>  void free_fragment(struct fragment *f);
>  bool is_ref_relative(const char *ref);
> +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);
>  
> diff --git a/livetree.c b/livetree.c
> index 3ff4e14..179866d 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -573,6 +573,32 @@ bool is_ref_relative(const char *ref)
>  	return ref[0] != '/' && strchr(&ref[1], '/');
>  }
>  
> +/*
> + * Merge a fragment which refers to a label into the node with that label. In
> + * a plugin, a label which is not found becomes an overlay fragment.
> + *
> + * Returns true on success, false on an error, which has been reported.
> + */
> +bool merge_ref(struct node *dt, struct fragment *f, bool plugin)
> +{
> +	struct node *target = get_node_by_ref(dt, f->ref);
> +
> +	assert(f->type == FRAGMENT_NODE);
> +	if (target) {
> +		merge_nodes(target, f->node);
> +		return true;
> +	}
> +	if (!plugin) {
> +		srcpos_error(f->ref_srcpos, "Error",
> +			     "Label or path %s not found", f->ref);
> +		return false;
> +	}
> +	/* The label may be in the base tree, so add a fragment@N node for it */
> +	add_orphan_node(dt, f->node, f->ref);
> +
> +	return true;
> +}
> +
>  /*
>   * Delete the node which a /delete-node/ fragment refers to
>   *
> -- 
> 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 --]