Re: [PATCH v3 09/15] libfdt: Handle unknown tags in fdt_next_tag()

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

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

On Thu, Sep 17, 2026 at 10:34:10AM +0200, Herve Codina wrote:
> Hi David,
> 
> On Wed, 16 Sep 2026 19:10:27 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Wed, Aug 26, 2026 at 10:31:40AM +0200, Herve Codina wrote:
> > > The structured tag value definition introduced recently gives the
> > > ability to ignore unknown tags without any error when they are read.
> > > 
> > > libfdt uses fdt_next_tag() to get a tag.
> > > 
> > > Filtering out tags that should be ignored in fdt_next_tag() allows to
> > > have the filtering done globally and allows, in future releases, to have
> > > a central place to add new known tags that should not be filtered out.
> > > 
> > > An already known tag exists with the meaning of "just ignore". This tag
> > > is FDT_NOP. fdt_next_tag() callers already handle the FDT_NOP tag.
> > > 
> > > Avoid unneeded modification at callers side and use a fake FDT_NOP tag
> > > when an unknown tag that should be ignored is encountered.
> > > 
> > > Add also fdt_next_tag_() internal function for callers who need to know
> > > if the FDT_NOP tag returned is a real FDT_NOP or a fake FDT_NOP due to
> > > an unknown tag.  
> > 
> > I'm a bit unsure about this one.  If we were designing the libfdt
> > interface from scratch, I'd definitely say that fdt_next_tag() - as a
> > low level function - should return every tag, and it's up to the
> > caller to skip unknown ones.  fdt_next_tag() already provides
> > nextoffset making it fairly easy to do so.
> > 
> > But, of course, we're not building from scratch so we do need to
> > consider backwards compatibility.  Supplying a skipping and
> > non-skipping version as you have here is the obvious approach.
> > 
> > But I'm not entirely convinced it's the only or best approach.  A user
> > which calls fdt_next_tag() is, in a sense, already opting in to
> > low-level handling of the dtb.  They could well already be broken by
> > new tags (depending on how they handle the default case).  Arguably
> > it's reasonable to require them to be updated (once only) for this new
> > family of tags.
> 
> I can go in that way. We will have same kind of sequence in almost all
> callers.
> 
>    do {
>       tag = fdt_next_tag()
>    } while (!fdt_tag_is_unknown(tag));

Not really - users of fdt_next_tag() will typically already have a
switch on the tag type, so the unknown tag handling would be an
addition to the 'default' case, not requiring an extra loop.

> We have to care about those "unknown_and_skippable" tags only when
> we modify a dtb. This is handled by a few functions.
> 
> The vast majority of callers of fdt_next_tag() just want to skip
> unknown tags.

I'm not sure that's true of the (few) callers outside libfdt itself.

> > Another potential approach would be to actually use the symbol
> > versioning we have, but have som far only minimally exploited.  The
> > LIBFDT_1.2 version of fdt_next_tag() would skip tags that postdate it,
> > but we'd introduce a new version tag and the new default version of
> > fdt_next_tag() would return all tags.  That would of course require
> > figuring out how to do that with the verison script.
> 
> Hum, I really don't want to have versioning in loop for fdt_next_tag().

I'm not sure what you mean by "in loop" here.

> Also, this could work in the dynamic lib (.so) case but not for static
> link and even less for libfdt code copy compiled in bootloaders or Linux
> kernel code.

Are symbol versions entirely unsupported with static libraries?
Obviously they'd have to be resolved at (static) link time, not
runtime, but I don't see that it's impossible.

> fdt_next_tag() is a low level function accessible out of libfdt (API).
> If it doesn't skip unknown tags even for other libfdt internal function it
> will be a too lower function.

I don't really follow what you're saying here.

> Ok, we can have a wrapper on top of
> fdt_next_tag() to skip unknown tags.
> 
> fdt_next_tag() is a low level function but the rule related to skipping
> unknown tags is also a low level rule.
> 
> We have to choose:
> a) fdt_next_tag() as I proposed
>    Of course ok for me
> 
> b) fdt_next_tag() returns all tags -> Full handling at caller side
>    Ok, we can go in that direction but it will bring quite a lot of
>    code duplication.

I'm not sure it's that bad, since fdt_next_tag() loops will typically
already have a 'default' case for irrelevant tags, which will
generally be handled the same as unknown tags.

> I am not fully convinced by this option.
> 
> c) lib version
>    I disagree on my side

Yeah, I'm not really convinced by this one either.

> d) Other idea
>    Of course, I am still open.

I haven't thought of any more ideas yet.

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