Re: [RFC PATCH 03/15] fdtdump: Return an error code on wrong tag value

David Gibson
2026-02-24T05:58:11Z

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

On Mon, Feb 23, 2026 at 09:39:50AM +0100, Herve Codina wrote:
> Hi David,
>
> On Mon, 23 Feb 2026 16:38:09 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
>
> > On Tue, Feb 10, 2026 at 06:33:31PM +0100, Herve Codina wrote:
> > > fdtdump prints a message on stderr when it encounters a wrong tag and
> > > stop its processing without returning an error code.
> > >
> > > Having a wrong tag is really a failure. Indeed, the processing cannot
> > > continue.
> > >
> > > Be more strict. Stop the processing, print a message and return an
> > > error code. In other words, call die().
> > >
> > > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> >
> > The intention of fdtdump is that it's a fairly crude debugging tool -
> > it will generally attempt to produce at least partial output even on a
> > bad dtb file. If you want a polished tool for use on good dtbs, use
> > :dtc -I dtb -O dts".
>
> fdtdump is also interesting for tests purpose.
> I use it to check dtb outputs during tests. Those outputs are either
> generated by dtc or by libfdt.

dtc -I dts should still be usable for that purpose, no? It seems like
the better tool for this job.

> Having an error code returned by fdtdump when it cannot parse the given dtb
> allows to have this test (patch 10):
> --- 8< ---
> +
> + base_run_test wrap_fdtdump unknown_tags_can_skip.dtb unknown_tags_can_skip.dtb.out
> + # Remove unneeded comments
> + sed -i '/^\/\/ /d' unknown_tags_can_skip.dtb.out
> + base_run_test check_diff unknown_tags_can_skip.dtb.out "$SRCDIR/unknown_tags_can_skip.dtb.expect"
> +
> + run_wrap_error_test $FDTDUMP unknown_tags_no_skip.dtb
> --- 8< ---
>
> >
> > That's why this didn't die() initially - the idea is if there's some
> > bogus stuff in the dtb, it might print a bunch of these warnings, but
> > eventually resynchronize on another valid tag.
>
> Current implementation cannot resynchronize. The parsing loop is exited
> https://git.kernel.org/pub/scm/utils/dtc/dtc.git/tree/fdtdump.c#n150
> and the program just returns 0.

Oh, good point. In that case this change is unequivocally an
improvement. Applied.

> Instead of just print and exit the loop, the idea was to have a program
> error code set. Calling die() allows to print, exit the loop and exit
> the program with an error code.
>
> With that in mind, I can do what you prefer. Either:
> - keep the die() call
> or
> - discard this patch and update the test in patch 10 (i.e. remove the
> 'run_wrap_error_test $FDTDUMP unknown_tags_no_skip.dtb' line)

So, I've applied the patch, so I guess option (1). However, I think
the test should be altered to use dtc instead of fdtdump. I don't
really encourage fdtdump to be used for anything except ad-hoc
experimentation.

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