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

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

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