Re: [PATCH 03/13] dtc: Move the /delete-node/ handling out of the grammar

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

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

On Sun, Nov 01, 2026 at 09:36:57AM -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. Add a fragment_ref rule
> to build this.
> 
> The output does not change.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  dtc-parser.y | 26 +++++++++++++++++---------
>  dtc.h        |  1 +
>  livetree.c   | 19 +++++++++++++++++++
>  3 files changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/dtc-parser.y b/dtc-parser.y
> index e47f80d..61301a6 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -47,6 +47,7 @@ static bool is_ref_relative(const char *ref)
>  	struct node *node;
>  	struct node *nodelist;
>  	struct reserve_info *re;
> +	struct fragment *fragment;
>  	uint64_t integer;
>  	unsigned int flags;
>  }
> @@ -80,6 +81,7 @@ static bool is_ref_relative(const char *ref)
>  %type <prop> propdef
>  %type <proplist> proplist
>  %type <labelref> dt_ref
> +%type <fragment> fragment_ref
>  
>  %type <node> devicetree
>  %type <node> nodedef
> @@ -157,6 +159,17 @@ memreserve:
>  
>  dt_ref: DT_LABEL_REF | DT_PATH_REF;
>  
> +fragment_ref:
> +	  DT_LABEL_REF
> +		{
> +			$$ = build_fragment_ref($1, false, &@$);
> +		}
> +	| DT_PATH_REF
> +		{
> +			$$ = build_fragment_ref($1, true, &@$);
> +		}
> +	;
> +

This seems weird.  If the point is to allow either kind of ref, we
already have the dt_ref nonterminal for that.  If the point is to
build fragments, rather than just refs, then why do it here before you
have the information you need to properly build the fragment?

>  devicetree:
>  	  '/' nodedef
>  		{
> @@ -236,16 +249,11 @@ devicetree:
>  			}
>  			$$ = $1;
>  		}
> -	| devicetree DT_DEL_NODE dt_ref ';'
> +	| devicetree DT_DEL_NODE fragment_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);
> -
> -
> +			$3->type = FRAGMENT_DELETE_NODE;
> +			if (merge_delete_node($1, $3))
> +				treesource_error = true;
>  			$$ = $1;
>  		}
>  	| devicetree DT_OMIT_NO_REF dt_ref ';'
> diff --git a/dtc.h b/dtc.h
> index c0d8e8f..9f9076d 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -368,6 +368,7 @@ struct fragment *build_fragment_ref(char *ref, bool path_ref,
>  				    struct srcpos *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 982f0a6..5c9ec48 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -542,6 +542,25 @@ struct dt_info *build_dt_info(unsigned int dtsflags,
>  	return dti;
>  }
>  
> +/*
> + * Delete the node which a /delete-node/ fragment refers to
> + *
> + * Returns true if there is 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() that f->type == DELETE_NODE, maybe?

> +	if (!target) {
> +		srcpos_error(f->ref_srcpos, "Error",
> +			     "Label or path %s not found", f->ref);
> +		return true;
> +	}
> +	delete_node(target);
> +
> +	return false;
> +}
> +
>  /*
>   * 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 --]