Re: [PATCH v3 02/15] libfdt: Don't assume the root node is available at offset 0

[David Gibson]

On Wed, Sep 09, 2026 at 08:58:26AM +0200, Herve Codina wrote:
> Hi David,
> 
> On Wed, 9 Sep 2026 16:18:22 +1000
> David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> 
> ...
> 
> > > 
> > > Having a fdt_root_offset() stop at either FDT_BEGIN_NODE_REF or FDT_BEGIN_NODE
> > > is "fdt_first_node_offset()".  
> > 
> > > Let me introduce the internal fdt_first_node_offset_()
> > > - fdt_root_offset()
> > >   It calls fdt_first_node_offset_() and check that this node is a
> > >   FDT_BEGIN_NODE node.
> > > 
> > > - fdt_check_node_offset_()
> > >   if the offset == 0, it updates the value with the offset returned by
> > >   fdt_first_node_offset_()  
> > 
> > Right, that's exactly what I'd suggest.
> > 
> > A possible tweak would be to have fdt_check_node_offset_() only apply
> > special case 0 handling if the _actual_ offset 0 doesn't look like a
> > valid node offset.  Not sure if that will make things messier or
> > cleaner.
> > 
> > > And so, a node offset 0 doesn't means the root node but the first node
> > > in the dtb (root or orphan). I am totally fine with this definition.  
> > 
> > Right.  If you don't want that behaviour for addon dtbs, then I think
> > the way to go would be to explicitly avoid all special case handling
> > of 0 if the header flags an addon.  Handling addons requires new code,
> > so we don't have to maintain backwards compatibliity for things that
> > used 0 assuming it meant root node.
> > 
> > > At some point, maybe users of the API (when addon are involved) will have to
> > > take care of that and perform something like:
> > >   root = fdt_root_offset();
> > >   fdt_get_property(fdt, root, "prop", NULL);
> > > 
> > > Or
> > >   fdt_for_each_orphan(orphan, fdt) {
> > >      fdt_get_property(fdt, orphan, "prop", NULL);
> > >      ...
> > >   }
> > > 
> > > Here also, I am totally fine with that an I already use this kind of sequence
> > > in libfdt/fdt_addon.c to apply an addon on a base dtb.
> > > 
> > > I will introduce fdt_first_node_offset_() but let me know if you prefer
> > > having fdt_first_node_offset_() introduced right now in this "structure
> > > tags" series or later in the addon series.  
> > 
> > Either is fine; do whichever results in less code churn.
> > 
> 
> Ok, I will do that.
> 
> Do you want to have an early version of that or having it in the
> next iteration of the series is fine on you side ?

Early is good.  Smaller series are easier to review, and getting them
fully sorted and merged also makes things easier.

> Of course, before sending a new iteration of the series, I am
> waiting for your feedback on the "structured tags" part. This part
> is not going to be impacted by the "offset 0 vs real root offset" we
> have discussed here and so what is available in this current series
> is still valid.

Right, the root offset stuff stands on its own, so it can sensibly be
split off.  Splitting series in such a way that they don't have enough
context to understand the wouldn't be helpful, of course.

-- 
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: signature.asc
Description: PGP signature