[PATCH v2 0/4] fdtput: Make it clearer how to create nodes

[PATCH v2 0/4] fdtput: Make it clearer how to create nodes

[Simon Glass]

fdtput can create nodes with -c, and -p creates any missing nodes along
the way, but this is easy to miss. A user has asked for a way to add
nodes, not realising that one exists.

The first patch makes -p reject a path without a leading '/', rather
than creating it from the root, as fdtput already does without -p. The
second allows such a path to start with an alias, creating nodes below
the node which the alias points to. The third suggests -p when a node
is missing, naming the parent which is missing. The fourth names the
property, as well as the node, when it cannot be deleted.

Link: https://github.com/dgibson/dtc/issues/49

Changes in v2:
- Add new patch to reject a path without a leading '/'
- Rework on top of the new patch which requires a leading '/'
- Drop the Fixes tag, since the new patch fixes the bug
- Check that the error suggests -p, using a new helper
- Drop David's Reviewed-by, since the tests have changed
- Report the node as well as the property
- Check the error message in the test
- Drop 2 patches previously applied

Simon Glass (4):
  fdtput: Require a leading '/' with -p
  fdtput: Follow an alias when creating nodes with -p
  fdtput: Suggest -p when a node is missing
  fdtput: Name the property when it cannot be deleted

 fdtput.c           | 41 ++++++++++++++++++++++++++++++-----
 tests/run_tests.sh | 54 ++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 88 insertions(+), 7 deletions(-)

---
base-commit: 1d50f3f8fa8bf9429eebb0769dafa5df37381ae1
branch: fdtput-nodes2

-- 
2.43.0

[PATCH v2 1/4] fdtput: Require a leading '/' with -p

[Simon Glass]

With -p, a path which does not start with '/' is created from the root,
so a mistyped path silently adds nodes at the top level:

   $ fdtput -cp test.dtb foo/bar

This creates /foo/bar. The same happens for a path which starts with an
alias, such as serial0/sub, which creates /serial0/sub rather than
adding to the node which the alias points to.

Without -p, fdtput rejects such a path, since fdt_path_offset() treats a
path without a leading '/' as starting with an alias:

   $ fdtput -c test.dtb foo/bar
   Error at 'foo': FDT_ERR_BADPATH

Report an error with -p too. The next patch allows such a path to start
with an alias.

This changes the behaviour of -p, so a script which relies on creating
nodes this way must add a leading '/'. Do this for the existing test
which relies on it, and add a test for the error.

Fixes: f807af192828 ("fdtput: Add -p option to create subnodes along entire path")
Suggested-by: David Gibson <david@gibson.dropbear.id.au>
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add new patch to reject a path without a leading '/'

 fdtput.c           | 9 +++++++--
 tests/run_tests.sh | 6 +++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/fdtput.c b/fdtput.c
index c139de1..d0f8420 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -201,8 +201,8 @@ static int store_key_value(char **blob, const char *node_name,
 /**
  * Create paths as needed for all components of a path
  *
- * Any components of the path that do not exist are created. Errors are
- * reported.
+ * Any components of the path that do not exist are created. The path
+ * must start with '/'. Errors are reported.
  *
  * @param blob		FDT blob to write into
  * @param in_path	Path to process
@@ -214,6 +214,11 @@ static int create_paths(char **blob, const char *in_path)
 	const char *sep;
 	int node, offset = 0;
 
+	if (*path != '/') {
+		report_error(path, -1, -FDT_ERR_BADPATH);
+		return -1;
+	}
+
 	/* skip leading '/' */
 	while (*path == '/')
 		path++;
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 842b543..644c405 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -970,8 +970,12 @@ fdtput_tests () {
 	"-ts" "fine wine"
     run_wrap_test $DTPUT $dtb -p /you/are/drunk/sir/winston slurp -ts twice
 
+    # A path must start with '/'
+    run_wrap_error_test $DTPUT $dtb -cp baldrick/turnip
+    run_wrap_error_test $DTGET $dtb -l /baldrick
+
     # Test expansion of the blob when insufficient room for a new node
-    run_wrap_test $DTPUT $dtb -cp "$(cat $text $text)/longish"
+    run_wrap_test $DTPUT $dtb -cp "/$(cat $text $text)/longish"
 
     # Allowed to create an existing node with -p
     run_wrap_test $DTPUT $dtb -cp /chosen
-- 
2.43.0

Re: [PATCH v2 1/4] fdtput: Require a leading '/' with -p

[Herve Codina]

On Wed, 28 Oct 2026 01:34:17 -0600
Simon Glass <sjg@chromium.org> wrote:

> With -p, a path which does not start with '/' is created from the root,
> so a mistyped path silently adds nodes at the top level:
> 
>    $ fdtput -cp test.dtb foo/bar
> 
> This creates /foo/bar. The same happens for a path which starts with an
> alias, such as serial0/sub, which creates /serial0/sub rather than
> adding to the node which the alias points to.
> 
> Without -p, fdtput rejects such a path, since fdt_path_offset() treats a
> path without a leading '/' as starting with an alias:
> 
>    $ fdtput -c test.dtb foo/bar
>    Error at 'foo': FDT_ERR_BADPATH
> 
> Report an error with -p too. The next patch allows such a path to start
> with an alias.

I would reword "The next patch allows" using "A future patch could allow" or
I would simply remove the whole sentence ("The next patch ...");

Indeed, this current patch can be applied without the next one.

Other than that:
Reviewed-by: Herve Codina <herve.codina@bootlin.com>

Best regards,
Hervé

> 
> This changes the behaviour of -p, so a script which relies on creating
> nodes this way must add a leading '/'. Do this for the existing test
> which relies on it, and add a test for the error.
> 
> Fixes: f807af192828 ("fdtput: Add -p option to create subnodes along entire path")
> Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---

Re: [PATCH v2 1/4] fdtput: Require a leading '/' with -p

[David Gibson]

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

On Wed, Oct 28, 2026 at 09:14:49PM +0100, Herve Codina wrote:
> On Wed, 28 Oct 2026 01:34:17 -0600
> Simon Glass <sjg@chromium.org> wrote:
> 
> > With -p, a path which does not start with '/' is created from the root,
> > so a mistyped path silently adds nodes at the top level:
> > 
> >    $ fdtput -cp test.dtb foo/bar
> > 
> > This creates /foo/bar. The same happens for a path which starts with an
> > alias, such as serial0/sub, which creates /serial0/sub rather than
> > adding to the node which the alias points to.
> > 
> > Without -p, fdtput rejects such a path, since fdt_path_offset() treats a
> > path without a leading '/' as starting with an alias:
> > 
> >    $ fdtput -c test.dtb foo/bar
> >    Error at 'foo': FDT_ERR_BADPATH
> > 
> > Report an error with -p too. The next patch allows such a path to start
> > with an alias.
> 
> I would reword "The next patch allows" using "A future patch could allow" or
> I would simply remove the whole sentence ("The next patch ...");

As a rule, that is a better way of phrasing a commit message, so that
it doesn't become inaccurate if this are reordered.  In this case it's
not worth a respin, though.

> 
> Indeed, this current patch can be applied without the next one.
> 
> Other than that:
> Reviewed-by: Herve Codina <herve.codina@bootlin.com>
> 
> Best regards,
> Hervé
> 
> > 
> > This changes the behaviour of -p, so a script which relies on creating
> > nodes this way must add a leading '/'. Do this for the existing test
> > which relies on it, and add a test for the error.
> > 
> > Fixes: f807af192828 ("fdtput: Add -p option to create subnodes along entire path")
> > Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> > Assisted-by: Claude:claude-opus-5-5
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> 

-- 
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 --]

Re: [PATCH v2 1/4] fdtput: Require a leading '/' with -p

[Simon Glass]

Hi David, Herve,

On Thu, 29 Oct 2026 at 02:07, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> On Wed, Oct 28, 2026 at 09:14:49PM +0100, Herve Codina wrote:
> > On Wed, 28 Oct 2026 01:34:17 -0600
> > Simon Glass <sjg@chromium.org> wrote:
> >
> > > With -p, a path which does not start with '/' is created from the root,
> > > so a mistyped path silently adds nodes at the top level:
> > >
> > >    $ fdtput -cp test.dtb foo/bar
> > >
> > > This creates /foo/bar. The same happens for a path which starts with an
> > > alias, such as serial0/sub, which creates /serial0/sub rather than
> > > adding to the node which the alias points to.
> > >
> > > Without -p, fdtput rejects such a path, since fdt_path_offset() treats a
> > > path without a leading '/' as starting with an alias:
> > >
> > >    $ fdtput -c test.dtb foo/bar
> > >    Error at 'foo': FDT_ERR_BADPATH
> > >
> > > Report an error with -p too. The next patch allows such a path to start
> > > with an alias.
> >
> > I would reword "The next patch allows" using "A future patch could allow" or
> > I would simply remove the whole sentence ("The next patch ...");
>
> As a rule, that is a better way of phrasing a commit message, so that
> it doesn't become inaccurate if this are reordered.  In this case it's
> not worth a respin, though.
>
> >
> > Indeed, this current patch can be applied without the next one.
> >
> > Other than that:
> > Reviewed-by: Herve Codina <herve.codina@bootlin.com>

Thanks both, yes that was better left out.

> >
> > Best regards,
> > Hervé
> >
> > >
> > > This changes the behaviour of -p, so a script which relies on creating
> > > nodes this way must add a leading '/'. Do this for the existing test
> > > which relies on it, and add a test for the error.
> > >
> > > Fixes: f807af192828 ("fdtput: Add -p option to create subnodes along entire path")
> > > Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> > > Assisted-by: Claude:claude-opus-5-5
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> >
[..]

Regards,
Simon

[PATCH v2 2/4] fdtput: Follow an alias when creating nodes with -p

[Simon Glass]

A node path can start with an alias, such as serial0/sub. fdtput allows
this when writing to an existing node, but -p requires the path to
start with '/', so it cannot create nodes below an alias:

   $ fdtput -cp test.dtb serial0/sub
   Error at 'serial0/sub': FDT_ERR_BADPATH

When the path does not start with '/', look up its first component as an
alias and create any missing nodes below the node which the alias points
to. A path which starts with neither '/' nor an alias is still
rejected.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Rework on top of the new patch which requires a leading '/'
- Drop the Fixes tag, since the new patch fixes the bug

 fdtput.c           | 17 +++++++++++++----
 tests/run_tests.sh |  9 ++++++++-
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/fdtput.c b/fdtput.c
index d0f8420..3c6cc98 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -201,8 +201,9 @@ static int store_key_value(char **blob, const char *node_name,
 /**
  * Create paths as needed for all components of a path
  *
- * Any components of the path that do not exist are created. The path
- * must start with '/'. Errors are reported.
+ * Any components of the path that do not exist are created. If the path
+ * starts with an alias, they are created below the node which the alias
+ * points to. Otherwise the path must start with '/'. Errors are reported.
  *
  * @param blob		FDT blob to write into
  * @param in_path	Path to process
@@ -214,9 +215,17 @@ static int create_paths(char **blob, const char *in_path)
 	const char *sep;
 	int node, offset = 0;
 
+	/* a path which does not start with '/' must start with an alias */
 	if (*path != '/') {
-		report_error(path, -1, -FDT_ERR_BADPATH);
-		return -1;
+		sep = strchr(path, '/');
+		if (!sep)
+			sep = path + strlen(path);
+		offset = fdt_path_offset_namelen(*blob, path, sep - path);
+		if (offset < 0) {
+			report_error(path, -1, offset);
+			return -1;
+		}
+		path = sep;
 	}
 
 	/* skip leading '/' */
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 644c405..b3d3f3a 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -970,7 +970,14 @@ fdtput_tests () {
 	"-ts" "fine wine"
     run_wrap_test $DTPUT $dtb -p /you/are/drunk/sir/winston slurp -ts twice
 
-    # A path must start with '/'
+    # Automatic node creation below an alias
+    run_wrap_test $DTPUT $dtb -p /aliases second -ts /blackadder/the-second
+    run_wrap_test $DTPUT $dtb -cp second/carrot
+    run_fdtput_test "orange" $dtb second/carrot colour -ts "orange"
+    run_fdtget_test "orange" $dtb /blackadder/the-second/carrot colour
+    run_wrap_error_test $DTGET $dtb -l /second
+
+    # A path must start with '/' or an alias
     run_wrap_error_test $DTPUT $dtb -cp baldrick/turnip
     run_wrap_error_test $DTGET $dtb -l /baldrick
 
-- 
2.43.0

Re: [PATCH v2 2/4] fdtput: Follow an alias when creating nodes with -p

[Herve Codina]

Hi Simon,

On Wed, 28 Oct 2026 01:34:18 -0600
Simon Glass <sjg@chromium.org> wrote:

> A node path can start with an alias, such as serial0/sub. fdtput allows
> this when writing to an existing node, but -p requires the path to
> start with '/', so it cannot create nodes below an alias:
> 
>    $ fdtput -cp test.dtb serial0/sub
>    Error at 'serial0/sub': FDT_ERR_BADPATH
> 
> When the path does not start with '/', look up its first component as an
> alias and create any missing nodes below the node which the alias points
> to. A path which starts with neither '/' nor an alias is still
> rejected.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 

LGTM.

Reviewed-by: Herve Codina <herve.codina@bootlin.com>

Best regards,
Hervé

[PATCH v2 3/4] fdtput: Suggest -p when a node is missing

[Simon Glass]

When the node being written to does not exist, fdtput just prints
FDT_ERR_NOTFOUND - nothing points to -p, which creates missing nodes, so
it is easy to conclude that fdtput cannot add nodes at all.

When creating a node with -c, the error also names the full path, even
though it is the parent which is missing.

Add a hint to use -p when a node is missing, both when writing a
property and when creating a node. Report the parent path when it is
the parent which is missing.

Add tests that writing a property or creating a node fails when a parent
node is missing and -p is not given, and that the error suggests -p.
Add a run_wrap_error_msg_test() helper for this, which checks the
output as well as the return code.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Check that the error suggests -p, using a new helper
- Drop David's Reviewed-by, since the tests have changed

 fdtput.c           | 20 ++++++++++++++++++--
 tests/run_tests.sh | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/fdtput.c b/fdtput.c
index 3c6cc98..b8d5081 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -47,6 +47,22 @@ static void report_error(const char *name, int namelen, int err)
 		fdt_strerror(err));
 }
 
+/**
+ * Report an error looking up a node which must exist before writing to it.
+ *
+ * If the node is missing, suggest -p, since it creates missing nodes.
+ *
+ * @param name		Node name to report error on
+ * @param namelen	Length of node name, or -1 to use entire string
+ * @param err		Error number to report (-FDT_ERR_...)
+ */
+static void report_node_error(const char *name, int namelen, int err)
+{
+	report_error(name, namelen, err);
+	if (err == -FDT_ERR_NOTFOUND)
+		fprintf(stderr, "Use -p to create missing nodes\n");
+}
+
 /**
  * Encode a series of arguments in a property value.
  *
@@ -182,7 +198,7 @@ static int store_key_value(char **blob, const char *node_name,
 
 	node = fdt_path_offset(*blob, node_name);
 	if (node < 0) {
-		report_error(node_name, -1, node);
+		report_node_error(node_name, -1, node);
 		return -1;
 	}
 
@@ -284,7 +300,7 @@ static int create_node(char **blob, const char *node_name)
 		node = fdt_path_offset(*blob, path);
 		free(path);
 		if (node < 0) {
-			report_error(node_name, -1, node);
+			report_node_error(node_name, p - node_name, node);
 			return -1;
 		}
 	}
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index b3d3f3a..cf978ee 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -181,6 +181,37 @@ run_wrap_error_test () {
     base_run_test wrap_error "$@"
 }
 
+# Run a command, checking that it fails and that its output has $1 in it
+wrap_error_msg () {
+    (
+	msg="$1"
+	shift
+	log=tmp.log.$$
+	trap "rm -f $log" 0
+	verbose_run_log "$log" "$@"
+	ret="$?"
+	FAIL_IF_SIGNAL $ret
+	if [ "$ret" -eq 0 ]; then
+	    FAIL "Expected non-zero return code"
+	fi
+	if ! grep -qF -- "$msg" "$log"; then
+	    FAIL "Expected message '$msg'"
+	fi
+	PASS
+    )
+}
+
+# Check that a command fails and prints a message
+# $1: message to look for in the output
+# $2...: command to run
+run_wrap_error_msg_test () {
+    msg="$1"
+    shift
+    shorten_echo "$@"
+    printf " {!= 0, \"%s\"}:	" "$msg"
+    base_run_test wrap_error_msg "$msg" "$@"
+}
+
 # $1: dtb file
 # $2: align base
 check_align () {
@@ -962,6 +993,12 @@ fdtput_tests () {
     run_wrap_error_test $DTPUT $dtb -c /chosen
     run_wrap_error_test $DTPUT $dtb -c /chosen/son
 
+    # Writing below a missing node fails without -p, suggesting -p
+    run_wrap_error_msg_test "Use -p to create missing nodes" \
+	$DTPUT $dtb -c /blackadder/the-second
+    run_wrap_error_msg_test "Use -p to create missing nodes" \
+	$DTPUT $dtb /blackadder/the-second cost -ts high
+
     # Automatic node creation
     run_wrap_test $DTPUT $dtb -cp /blackadder/the-second/turnip \
 	/blackadder/the-second/potato
-- 
2.43.0

Re: [PATCH v2 3/4] fdtput: Suggest -p when a node is missing

[Herve Codina]

Hi Simon,

On Wed, 28 Oct 2026 01:34:19 -0600
Simon Glass <sjg@chromium.org> wrote:

> When the node being written to does not exist, fdtput just prints
> FDT_ERR_NOTFOUND - nothing points to -p, which creates missing nodes, so
> it is easy to conclude that fdtput cannot add nodes at all.
> 
> When creating a node with -c, the error also names the full path, even
> though it is the parent which is missing.
> 
> Add a hint to use -p when a node is missing, both when writing a
> property and when creating a node. Report the parent path when it is
> the parent which is missing.
> 
> Add tests that writing a property or creating a node fails when a parent
> node is missing and -p is not given, and that the error suggests -p.
> Add a run_wrap_error_msg_test() helper for this, which checks the
> output as well as the return code.

Nice idea!

> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v2:
> - Check that the error suggests -p, using a new helper
> - Drop David's Reviewed-by, since the tests have changed
> 
>  fdtput.c           | 20 ++++++++++++++++++--
>  tests/run_tests.sh | 37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+), 2 deletions(-)
> 
> diff --git a/fdtput.c b/fdtput.c
> index 3c6cc98..b8d5081 100644
> --- a/fdtput.c
> +++ b/fdtput.c

...

I am ok with the implementation in fdtput.c.


> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index b3d3f3a..cf978ee 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -181,6 +181,37 @@ run_wrap_error_test () {
>      base_run_test wrap_error "$@"
>  }
>  
> +# Run a command, checking that it fails and that its output has $1 in it
> +wrap_error_msg () {
> +    (
> +	msg="$1"
> +	shift
> +	log=tmp.log.$$
> +	trap "rm -f $log" 0
> +	verbose_run_log "$log" "$@"
> +	ret="$?"
> +	FAIL_IF_SIGNAL $ret
> +	if [ "$ret" -eq 0 ]; then
> +	    FAIL "Expected non-zero return code"
> +	fi
> +	if ! grep -qF -- "$msg" "$log"; then
> +	    FAIL "Expected message '$msg'"
> +	fi
> +	PASS
> +    )
> +}
> +
> +# Check that a command fails and prints a message
> +# $1: message to look for in the output
> +# $2...: command to run
> +run_wrap_error_msg_test () {
> +    msg="$1"
> +    shift
> +    shorten_echo "$@"
> +    printf " {!= 0, \"%s\"}:	" "$msg"
> +    base_run_test wrap_error_msg "$msg" "$@"
> +}
> +

Right, you introduce this wrapper and I think they can be useful for other
cases.

You also sent the series "dtc: Fix deletes within a section and before a
redefinition" [1].

The patch "tests: Allow dtc-fatal.sh to check the error message" [2]
available in the series plays also with error messages, adding a "-m" option
to tests/dtc-fatal.sh.

At some point, could all tests use the same uniform method (probably the
wrappers added here)?

Could dtc-fatal.sh script be replaced by an existing wrapper?

Again, I think wrappers you added here are nice. I would like to have your
opinion related to dtc-fatal.sh and a way to avoid the '-m' option in order
to have uniform features.

[1]: https://lore.kernel.org/devicetree-compiler/20261028093620.3705074-1-sjg@chromium.org/
[2]: https://lore.kernel.org/devicetree-compiler/20261028093620.3705074-5-sjg@chromium.org/

>  # $1: dtb file
>  # $2: align base
>  check_align () {
> @@ -962,6 +993,12 @@ fdtput_tests () {
>      run_wrap_error_test $DTPUT $dtb -c /chosen
>      run_wrap_error_test $DTPUT $dtb -c /chosen/son
>  
> +    # Writing below a missing node fails without -p, suggesting -p
> +    run_wrap_error_msg_test "Use -p to create missing nodes" \
> +	$DTPUT $dtb -c /blackadder/the-second
> +    run_wrap_error_msg_test "Use -p to create missing nodes" \
> +	$DTPUT $dtb /blackadder/the-second cost -ts high
> +
>      # Automatic node creation
>      run_wrap_test $DTPUT $dtb -cp /blackadder/the-second/turnip \
>  	/blackadder/the-second/potato

Best regards,
Hervé

Re: [PATCH v2 3/4] fdtput: Suggest -p when a node is missing

[David Gibson]

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

On Wed, Oct 28, 2026 at 10:08:54PM +0100, Herve Codina wrote:
> Hi Simon,
> 
> On Wed, 28 Oct 2026 01:34:19 -0600
> Simon Glass <sjg@chromium.org> wrote:
> 
> > When the node being written to does not exist, fdtput just prints
> > FDT_ERR_NOTFOUND - nothing points to -p, which creates missing nodes, so
> > it is easy to conclude that fdtput cannot add nodes at all.
> > 
> > When creating a node with -c, the error also names the full path, even
> > though it is the parent which is missing.
> > 
> > Add a hint to use -p when a node is missing, both when writing a
> > property and when creating a node. Report the parent path when it is
> > the parent which is missing.
> > 
> > Add tests that writing a property or creating a node fails when a parent
> > node is missing and -p is not given, and that the error suggests -p.
> > Add a run_wrap_error_msg_test() helper for this, which checks the
> > output as well as the return code.
> 
> Nice idea!
> 
> > 
> > Assisted-by: Claude:claude-opus-5-5
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> > 
> > Changes in v2:
> > - Check that the error suggests -p, using a new helper
> > - Drop David's Reviewed-by, since the tests have changed
> > 
> >  fdtput.c           | 20 ++++++++++++++++++--
> >  tests/run_tests.sh | 37 +++++++++++++++++++++++++++++++++++++
> >  2 files changed, 55 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fdtput.c b/fdtput.c
> > index 3c6cc98..b8d5081 100644
> > --- a/fdtput.c
> > +++ b/fdtput.c
> 
> ...
> 
> I am ok with the implementation in fdtput.c.
> 
> 
> > diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> > index b3d3f3a..cf978ee 100755
> > --- a/tests/run_tests.sh
> > +++ b/tests/run_tests.sh
> > @@ -181,6 +181,37 @@ run_wrap_error_test () {
> >      base_run_test wrap_error "$@"
> >  }
> >  
> > +# Run a command, checking that it fails and that its output has $1 in it
> > +wrap_error_msg () {
> > +    (
> > +	msg="$1"
> > +	shift
> > +	log=tmp.log.$$
> > +	trap "rm -f $log" 0
> > +	verbose_run_log "$log" "$@"
> > +	ret="$?"
> > +	FAIL_IF_SIGNAL $ret
> > +	if [ "$ret" -eq 0 ]; then
> > +	    FAIL "Expected non-zero return code"
> > +	fi
> > +	if ! grep -qF -- "$msg" "$log"; then
> > +	    FAIL "Expected message '$msg'"
> > +	fi
> > +	PASS
> > +    )
> > +}
> > +
> > +# Check that a command fails and prints a message
> > +# $1: message to look for in the output
> > +# $2...: command to run
> > +run_wrap_error_msg_test () {
> > +    msg="$1"
> > +    shift
> > +    shorten_echo "$@"
> > +    printf " {!= 0, \"%s\"}:	" "$msg"
> > +    base_run_test wrap_error_msg "$msg" "$@"
> > +}
> > +
> 
> Right, you introduce this wrapper and I think they can be useful for other
> cases.
> 
> You also sent the series "dtc: Fix deletes within a section and before a
> redefinition" [1].
> 
> The patch "tests: Allow dtc-fatal.sh to check the error message" [2]
> available in the series plays also with error messages, adding a "-m" option
> to tests/dtc-fatal.sh.
> 
> At some point, could all tests use the same uniform method (probably the
> wrappers added here)?
> 
> Could dtc-fatal.sh script be replaced by an existing wrapper?
> 
> Again, I think wrappers you added here are nice. I would like to have your
> opinion related to dtc-fatal.sh and a way to avoid the '-m' option in order
> to have uniform features.

Yes, it would be good to consolidate.  For now, this series is ready
and the other still needs review, so I think we make it adjust to the
changes from this one, rather than the other way around.

-- 
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 --]

[PATCH v2 4/4] fdtput: Name the property when it cannot be deleted

[Simon Glass]

When 'fdtput -d' fails to delete a property, for example because it does
not exist, the error names the node rather than the property:

   $ fdtput -d test.dtb /chosen missing
   Error at '/chosen': FDT_ERR_NOTFOUND

This suggests that the node is missing, when it is the property that
is.

Report the property name as well as the node:

   $ fdtput -d test.dtb /chosen missing
   Error at '/chosen', property 'missing': FDT_ERR_NOTFOUND

Check the message in the existing test for this.

Fixes: aa719618a8b7 ("fdtput: add delete node and property function")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Report the node as well as the property
- Check the error message in the test

 fdtput.c           | 3 ++-
 tests/run_tests.sh | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/fdtput.c b/fdtput.c
index b8d5081..610cad4 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -334,7 +334,8 @@ static int delete_prop(char *blob, const char *node_name, const char *prop_name)
 
 	node = fdt_delprop(blob, node, prop_name);
 	if (node < 0) {
-		report_error(node_name, -1, node);
+		fprintf(stderr, "Error at '%s', property '%s': %s\n",
+			node_name, prop_name, fdt_strerror(node));
 		return -1;
 	}
 
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index cf978ee..5bc7817 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -1045,7 +1045,9 @@ fdtput_tests () {
     run_fdtget_test "bootargs\nlinux,platform" $dtb -p  /chosen
 
     # Delete the non-existent property
-    run_wrap_error_test $DTPUT $dtb -d /chosen   non-existent-prop
+    run_wrap_error_msg_test \
+	"Error at '/chosen', property 'non-existent-prop'" \
+	$DTPUT $dtb -d /chosen non-existent-prop
 
     # TODO: Add tests for verbose mode?
 }
-- 
2.43.0

Re: [PATCH v2 4/4] fdtput: Name the property when it cannot be deleted

[Herve Codina]

Hi Simon,

On Wed, 28 Oct 2026 01:34:20 -0600
Simon Glass <sjg@chromium.org> wrote:

> When 'fdtput -d' fails to delete a property, for example because it does
> not exist, the error names the node rather than the property:
> 
>    $ fdtput -d test.dtb /chosen missing
>    Error at '/chosen': FDT_ERR_NOTFOUND
> 
> This suggests that the node is missing, when it is the property that
> is.
> 
> Report the property name as well as the node:
> 
>    $ fdtput -d test.dtb /chosen missing
>    Error at '/chosen', property 'missing': FDT_ERR_NOTFOUND
> 
> Check the message in the existing test for this.
> 
> Fixes: aa719618a8b7 ("fdtput: add delete node and property function")
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---

This patch will depends on discussion on previous one.

If test wrappers introduced previously are accepted (IHMO again, they are
good), for this current patch:
Reviewed-by: Herve Codina <herve.codina@bootlin.com>

Best regards,
Hervé

Re: [PATCH v2 0/4] fdtput: Make it clearer how to create nodes

[David Gibson]

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

On Wed, Oct 28, 2026 at 01:34:16AM -0600, Simon Glass wrote:
> fdtput can create nodes with -c, and -p creates any missing nodes along
> the way, but this is easy to miss. A user has asked for a way to add
> nodes, not realising that one exists.
> 
> The first patch makes -p reject a path without a leading '/', rather
> than creating it from the root, as fdtput already does without -p. The
> second allows such a path to start with an alias, creating nodes below
> the node which the alias points to. The third suggests -p when a node
> is missing, naming the parent which is missing. The fourth names the
> property, as well as the node, when it cannot be deleted.
> 
> Link: https://github.com/dgibson/dtc/issues/49

Merged, thanks.

-- 
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 --]