On Thu, Oct 24, 2019 at 09:29:25PM -0600, Simon Glass wrote:
> This function is used to perform a full check of the device tree. Allow
> it to be excluded if all assumptions are enabled.
>
> Signed-off-by: Simon Glass <sjg@xxxxxxxxxxxx>
Allowing this function to be excluded is an obvious thing, but I'm not
sure the conditions for doing so here are quite right.
I think entirely excludiing the function, rather than stubbing it *is*
the right choice, even though that changes the API. Better to have a
compile (well, link) error if you attempt fdt_check_full() than to
call that function and it not actually do a full check.
On that note, I wonder if it should be excluded if we have *any*
assumptions active which compromise its checking. Do your other
changes alter fdt_check_header() or fdt_next_tag() or other functions
this calls in a way that will reduce the strength of its checks? If
so it should maybe be excluded in those cases as well.
Then again, if we can arrange for this to do full checks, even if
other paths aren't checked, we have another possibility. To allow for
code to do an fdt_check_full() first, *then* assume the fdt is all
correct for future calls. That might be a reasonable safety / space
tradeoff for certain cases.
We could consider moving this to another .c file, or using
-ffunction-sections to allow this to be excluded from the link without
#if-ing it out entirely.
> ---
>
> Changes in v3:
> - Rearrange code in terms of checks instead of files changed, to aid review
> - Update commit message a little
>
> Changes in v2:
> - Add a comment to fdt_find_add_string_()
> - Correct inverted version checks in a few cases
> - Drop optimised code path in fdt_nodename_eq_()
> - Update to use new check functions
> - Use fdt_chk_base() in fdt_blocks_misordered_()
>
> libfdt/fdt_ro.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
> index 723f857..e398815 100644
> --- a/libfdt/fdt_ro.c
> +++ b/libfdt/fdt_ro.c
> @@ -855,6 +855,7 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
> return offset; /* error from fdt_next_node() */
> }
>
> +#if !defined(FDT_ASSUME_MASK) || FDT_ASSUME_MASK != 0xff
> int fdt_check_full(const void *fdt, size_t bufsize)
> {
> int err;
> @@ -917,3 +918,4 @@ int fdt_check_full(const void *fdt, size_t bufsize)
> }
> }
> }
> +#endif
--
David Gibson | 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
