Re: [PATCH] libfdt: fdt_check_full: Add can_assume(PERFECT) check

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

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

On Tue, May 26, 2026 at 02:30:22PM -0600, Tom Rini wrote:
> In this function from fdt_check.c we have (reasonably and as the name
> implies) a number of checks on the DTB. However, there are cases where
> we may wish to assume that we have been given a perfect DTB already and
> do nothing here. Add a test for can_assume(PERFECT) as the first check
> in this function and if true, perform no checks.
> 
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
> Along the lines of the patches I posted back in December, in U-Boot SPL
> we just don't have the space for this check much of the time and so have
> always omitted it (going back to at least when Simon posted the initial
> patch to make libfdt/fdt_check.c here). This is another case where it's
> a noticeable size win for us. I had missed this change in particular
> because we had in turn missed catching up on fdt_check_full being moved
> out of fdt_ro.c and in to fdt_check.c.

I'm not necessarily against this, but I have some misgivings.

fdt_check_full() is (deliberately) not called from anywhere else in
libfdt - it's intended to allow the user to explicitly do a full
validity check on the tree.  Given that meaning, I'm not sure it's
wise to turn it into a no-op based on the assume flags.

Your comment seems to imply that the issue here is size - simply
having this function compiled - rather than being too expensive when
(explicitly) called.  That's a little surprising to me - it's in its
own compilation unit, specifically so that the linker can omit it if
it's not used.  Is there something unusual about your build
environment that's not letting that happen?

> ---
>  libfdt/fdt_check.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libfdt/fdt_check.c b/libfdt/fdt_check.c
> index cca052353213..2fd5b61d016a 100644
> --- a/libfdt/fdt_check.c
> +++ b/libfdt/fdt_check.c
> @@ -21,6 +21,8 @@ int fdt_check_full(const void *fdt, size_t bufsize)
>  	const char *propname;
>  	bool expect_end = false;
>  
> +	if (can_assume(PERFECT))
> +		return 0;
>  	if (bufsize < FDT_V1_SIZE)
>  		return -FDT_ERR_TRUNCATED;
>  	if (bufsize < fdt_header_size(fdt))
> -- 
> 2.43.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 --]