Re: [PATCH v3 01/15] fdtget: Use libfdt iterators instead of open coded loops

[David Gibson]

On Wed, Aug 26, 2026 at 10:31:32AM +0200, Herve Codina wrote:
> fdtget uses directly fdt_{first,next}_property_offset() with a while(1)
> loop to iterate over node properties.
> 
> It also uses the low level primitive fdt_next_tag() with custom tags,
> level and depth handling to iterates over subnodes.
> 
> It is worth noting that FDT_NOP can be returned by fdt_next_tag() and
> this tag is not taken into account in the fdtget open coded loop. This
> will lead to an incorrect error if a FDT_NOP tag is encountered.
> 
> libfdt provides iterators to iterate over node properties and subnodes.
> The subnode iterator provided by libfdt is robust against FDT_NOP tags
> and will be robust in the future when new tags are introduced.
> 
> Replace fdtget open coded loops by iterators provided by libfdt and
> designed to perform those operations.
> 
> Signed-off-by: Herve Codina <herve.codina@xxxxxxxxxxx>

Nice cleanup, merged.  A test case to prevent any regressions on that
FDT_NOP bug would be a nice addition.

> ---
>  fdtget.c | 73 ++++++++++++++++++--------------------------------------
>  1 file changed, 23 insertions(+), 50 deletions(-)
> 
> diff --git a/fdtget.c b/fdtget.c
> index dd709854..c6169691 100644
> --- a/fdtget.c
> +++ b/fdtget.c
> @@ -138,21 +138,20 @@ static int show_data(struct display_info *disp, const char *data, int len)
>  static int list_properties(const void *blob, int node)
>  {
>  	const char *name;
> +	const void *p;
>  	int prop;
>  
> -	prop = fdt_first_property_offset(blob, node);
> -	do {
> -		/* Stop silently when there are no more properties */
> -		if (prop < 0)
> -			return prop == -FDT_ERR_NOTFOUND ? 0 : prop;
> -		fdt_getprop_by_offset(blob, prop, &name, NULL);
> -		if (name)
> +	fdt_for_each_property_offset(prop, blob, node) {
> +		p = fdt_getprop_by_offset(blob, prop, &name, NULL);
> +		if (p && name)
>  			puts(name);
> -		prop = fdt_next_property_offset(blob, prop);
> -	} while (1);
> -}
> +	}
>  
> -#define MAX_LEVEL	32		/* how deeply nested we will go */
> +	if ((prop < 0) && (prop != -FDT_ERR_NOTFOUND))
> +		return prop;
> +
> +	return 0;
> +}
>  
>  /**
>   * List all subnodes in a node, one per line
> @@ -163,47 +162,21 @@ static int list_properties(const void *blob, int node)
>   */
>  static int list_subnodes(const void *blob, int node)
>  {
> -	int nextoffset;		/* next node offset from libfdt */
> -	uint32_t tag;		/* current tag */
> -	int level = 0;		/* keep track of nesting level */
>  	const char *pathp;
> -	int depth = 1;		/* the assumed depth of this node */
> -
> -	while (level >= 0) {
> -		tag = fdt_next_tag(blob, node, &nextoffset);
> -		switch (tag) {
> -		case FDT_BEGIN_NODE:
> -			pathp = fdt_get_name(blob, node, NULL);
> -			if (level <= depth) {
> -				if (pathp == NULL)
> -					pathp = "/* NULL pointer error */";
> -				if (*pathp == '\0')
> -					pathp = "/";	/* root is nameless */
> -				if (level == 1)
> -					puts(pathp);
> -			}
> -			level++;
> -			if (level >= MAX_LEVEL) {
> -				printf("Nested too deep, aborting.\n");
> -				return 1;
> -			}
> -			break;
> -		case FDT_END_NODE:
> -			level--;
> -			if (level == 0)
> -				level = -1;		/* exit the loop */
> -			break;
> -		case FDT_END:
> -			return 1;
> -		case FDT_PROP:
> -			break;
> -		default:
> -			if (level <= depth)
> -				printf("Unknown tag 0x%08X\n", tag);
> -			return 1;
> -		}
> -		node = nextoffset;
> +	int subnode;
> +
> +	fdt_for_each_subnode(subnode, blob, node) {
> +		pathp = fdt_get_name(blob, subnode, NULL);
> +		if (pathp == NULL)
> +			pathp = "/* NULL pointer error */";
> +		if (*pathp == '\0')
> +			pathp = "/";	/* root is nameless */
> +		puts(pathp);
>  	}
> +
> +	if (subnode < 0 && (subnode != -FDT_ERR_NOTFOUND))
> +		return subnode;
> +
>  	return 0;
>  }
>  
> -- 
> 2.55.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: signature.asc
Description: PGP signature