Re: [RFC PATCH 03/77] libfdt: Introduce fdt_next_tag_full() and use it in fdt_next_tag()

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

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

On Mon, Jan 12, 2026 at 03:18:53PM +0100, Herve Codina wrote:
> In v18 dtb new tags are added. Prepare libfdt to handle those new tags.
> 
> Keep fdt_next_tag() handling only existing tags and introduce
> fdt_next_tag_full() to handle new tags.

Ugh, that's super ugly.

My first inclination is to say that if you're using the low-level
fdt_next_tag() interface, it's your responsibility to check the
version first - fdt_next_tag() should return... the next tag... and
it's the caller's problem if it doesn't know about it.

Maybe that breaks some real user too badly, though.  The other
approach would be to use the symbol versioning we already have: we
bump the libfdt version, and have different versions of fdt_next_tag()
depending on the version you're using.

Btw, once you generate a dtb with new tags, its last_compat_version
must become v18 as well as it's version - tools need to be aware of
the new tags.

> 
> fdt_next_tag() uses fdt_next_tag_full() but it will filter out new tags
> when they are introduced to have those new tags transparent for existing
> fdt_next_tag() callers.
> 
> Code that will need to handle those new tags will use explicitly
> fdt_next_tag_full() to have access to them when they need to.
> 
> No new tags have been introduced yet and modifications done here prepare
> their introduction.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  libfdt/fdt.c       | 35 ++++++++++++++++++++++++++++++++++-
>  libfdt/libfdt.h    | 18 ++++++++++++++++++
>  libfdt/version.lds |  1 +
>  3 files changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> index 95f644c..ce051a0 100644
> --- a/libfdt/fdt.c
> +++ b/libfdt/fdt.c
> @@ -159,7 +159,7 @@ const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
>  	return fdt_offset_ptr_(fdt, offset);
>  }
>  
> -uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
> +uint32_t fdt_next_tag_full(const void *fdt, int startoffset, int *nextoffset)
>  {
>  	const fdt32_t *tagp, *lenp;
>  	uint32_t tag, len, sum;
> @@ -220,6 +220,39 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
>  	return tag;
>  }
>  
> +uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
> +{
> +	uint32_t tag, tmp_tag;
> +	int tmp_offset, tmp_next;
> +
> +	/* Retrieve next tag */
> +	tag = fdt_next_tag_full(fdt, startoffset, nextoffset);
> +
> +	/* Look at next one to see what we need to do */
> +	tmp_next = *nextoffset;
> +	do {
> +		tmp_offset = tmp_next;
> +		tmp_tag = fdt_next_tag_full(fdt, tmp_offset, &tmp_next);
> +		switch (tmp_tag) {
> +		case FDT_BEGIN_NODE:
> +		case FDT_END_NODE:
> +		case FDT_PROP:
> +		case FDT_NOP:
> +		case FDT_END:
> +			/* Next tag is not new tag introduced in v18 -> Ok */
> +			*nextoffset = tmp_offset;
> +			return tag;
> +
> +		default:
> +			break;
> +		}
> +	} while (1);
> +
> +	/* We shouldn't reach this code */
> +	*nextoffset = -FDT_ERR_BADSTRUCTURE;
> +	return FDT_END;
> +}
> +
>  int fdt_check_node_offset_(const void *fdt, int offset)
>  {
>  	if (!can_assume(VALID_INPUT)
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index c5cd35d..d1a9cd5 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -154,6 +154,24 @@ static inline void *fdt_offset_ptr_w(void *fdt, int offset, int checklen)
>   */
>  uint32_t fdt_next_tag(const void *fdt, int offset, int *nextoffset);
>  
> +/**
> + * fdt_next_tag_full - get next tag in the device tree without any filtering
> + * @fdt:	Pointer to the device tree blob
> + * @offset:	Offset within the blob to start searching
> + * @nextoffset:	Pointer to variable to store the offset of the next tag
> + *
> + * fdt_next_tag_full() returns the tag type of the next tag in the device tree
> + * blob starting from the given @offset. If @nextoffset is non-NULL, it will
> + * be set to the offset immediately following the tag.
> + * fdt_next_tag() can return only a subset of all possible tags performing some
> + * internal filtering. fdt_next_tag_full() doesn't perform this filtering.
> + *
> + * returns:
> + *	the tag type (FDT_BEGIN_NODE, FDT_END_NODE, FDT_PROP, FDT_NOP, FDT_END),
> + *	FDT_END, if offset is out of bounds
> + */
> +uint32_t fdt_next_tag_full(const void *fdt, int offset, int *nextoffset);
> +
>  /*
>   * External helpers to access words from a device tree blob. They're built
>   * to work even with unaligned pointers on platforms (such as ARMv5) that don't
> diff --git a/libfdt/version.lds b/libfdt/version.lds
> index cbfef54..7e2dde2 100644
> --- a/libfdt/version.lds
> +++ b/libfdt/version.lds
> @@ -52,6 +52,7 @@ LIBFDT_1.2 {
>  		fdt_strerror;
>  		fdt_offset_ptr;
>  		fdt_next_tag;
> +		fdt_next_tag_full;
>  		fdt_appendprop;
>  		fdt_create_empty_tree;
>  		fdt_first_property_offset;
> -- 
> 2.52.0
> 
> 

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