On Tue, Nov 12, 2019 at 06:54:17PM -0700, Simon Glass wrote:
> There does not seem to be a strong reason to inline this function.
Well.. I think my rationale was just that this function should be a
single load, followed by the call to fdt_header_size_(). I suspect
it's not called enough for removing that to balance out the code
overhead of an extra function. Or have you made measurements which
show I'm wrong about that?
> Also
> we are about to add some extra code to it which will increase its size.
Will they, though? As far as I can tell, the only changes you make to
this function in this series should be resolved at compile time. So
that it should either be identical object code to what we have now, or
even simpler (in the assume case it will be returning a fixed value,
so in fact you'd probably get even better code size by allowing that
to be inlined).
> Move it into fdt.c and use a simple declaration in libfdt.h
>
> Signed-off-by: Simon Glass <sjg@xxxxxxxxxxxx>
> ---
>
> Changes in v4:
> - Add fdt_header_size to version.lds
>
> Changes in v3:
> - Add a new patch to de-inline fdt_header_size()
>
> Changes in v2: None
>
> libfdt/fdt.c | 5 +++++
> libfdt/libfdt.h | 9 +++++----
> libfdt/version.lds | 1 +
> tests/testutils.c | 1 -
> 4 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/libfdt/fdt.c b/libfdt/fdt.c
> index d6ce7c0..3e37a4b 100644
> --- a/libfdt/fdt.c
> +++ b/libfdt/fdt.c
> @@ -70,6 +70,11 @@ size_t fdt_header_size_(uint32_t version)
> return FDT_V17_SIZE;
> }
>
> +size_t fdt_header_size(const void *fdt)
> +{
> + return fdt_header_size_(fdt_version(fdt));
> +}
> +
> int fdt_check_header(const void *fdt)
> {
> size_t hdrsize;
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index fc4c496..48f375c 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -266,11 +266,12 @@ fdt_set_hdr_(size_dt_struct);
> * fdt_header_size - return the size of the tree's header
> * @fdt: pointer to a flattened device tree
> */
> +size_t fdt_header_size(const void *fdt);
> +
> +/**
> + * fdt_header_size_ - internal function which takes a version number
> + */
> size_t fdt_header_size_(uint32_t version);
> -static inline size_t fdt_header_size(const void *fdt)
> -{
> - return fdt_header_size_(fdt_version(fdt));
> -}
>
> /**
> * fdt_check_header - sanity check a device tree header
> diff --git a/libfdt/version.lds b/libfdt/version.lds
> index ae32924..7ab85f1 100644
> --- a/libfdt/version.lds
> +++ b/libfdt/version.lds
> @@ -20,6 +20,7 @@ LIBFDT_1.2 {
> fdt_get_alias_namelen;
> fdt_get_alias;
> fdt_get_path;
> + fdt_header_size;
> fdt_supernode_atdepth_offset;
> fdt_node_depth;
> fdt_parent_offset;
> diff --git a/tests/testutils.c b/tests/testutils.c
> index 5e494c5..53cb35f 100644
> --- a/tests/testutils.c
> +++ b/tests/testutils.c
> @@ -261,7 +261,6 @@ void vg_prepare_blob(void *fdt, size_t bufsize)
>
> VALGRIND_MAKE_MEM_UNDEFINED(blob, bufsize);
> VALGRIND_MAKE_MEM_DEFINED(blob, FDT_V1_SIZE);
> - VALGRIND_MAKE_MEM_DEFINED(blob, fdt_header_size(fdt));
>
> if (fdt_magic(fdt) == FDT_MAGIC) {
> off_strings = fdt_off_dt_strings(fdt);
--
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
