Re: [PATCH v3 04/18] dtc: Move the /delete-node/ handling out of the grammar

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

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

On Fri, Nov 06, 2026 at 12:51:31PM -0700, Simon Glass wrote:
> The grammar actions merge each top-level fragment of a dts file into the
> tree as it is parsed, so the merge logic is spread over the rules for
> each kind of fragment. This makes it hard to follow and to change.
> 
> As a first step towards moving this logic into livetree.c, move the
> action for a top-level /delete-node/ into a new merge_delete_node(),
> passing it a struct fragment for the reference.
> 
> The output does not change.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>

LGTM, except for any rework needed to accomodate changes suggested in
earlier patches.


> ---
> 
> (no changes since v2)
> 
> Changes in v2:
> - Use dt_ref instead of fragment_ref
> - Return true on success from merge_delete_node()
> - Check the fragment type with assert()
> 
>  dtc-parser.y | 11 ++++-------
>  dtc.h        |  1 +
>  livetree.c   | 20 ++++++++++++++++++++
>  3 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index e47f80d..bba98b6 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -238,14 +238,11 @@ devicetree:
>  		}
>  	| devicetree DT_DEL_NODE dt_ref ';'
>  		{
> -			struct node *target = get_node_by_ref($1, $3);
> -
> -			if (target)
> -				delete_node(target);
> -			else
> -				ERROR(&@3, "Label or path %s not found", $3);
> -
> +			struct fragment *f;
>  
> +			f = build_fragment_delete_node($3, &@3);
> +			if (!merge_delete_node($1, f))
> +				treesource_error = true;
>  			$$ = $1;
>  		}
>  	| devicetree DT_OMIT_NO_REF dt_ref ';'
> diff --git a/dtc.h b/dtc.h
> index f15a162..5dca8bc 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -372,6 +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_delete_node(struct node *dt, struct fragment *f);
>  
>  struct dt_info {
>  	unsigned int dtsflags;
> diff --git a/livetree.c b/livetree.c
> index 85725d3..35beee0 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -567,6 +567,26 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
>  	return dti;
>  }
>  
> +/*
> + * Delete the node which a /delete-node/ fragment refers to
> + *
> + * Returns true on success, false on an error, which has been reported.
> + */
> +bool merge_delete_node(struct node *dt, struct fragment *f)
> +{
> +	struct node *target = get_node_by_ref(dt, f->ref);
> +
> +	assert(f->type == FRAGMENT_DELETE_NODE);
> +	if (!target) {
> +		srcpos_error(f->ref_srcpos, "Error",
> +			     "Label or path %s not found", f->ref);
> +		return false;
> +	}
> +	delete_node(target);
> +
> +	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 --]