Re: [PATCH 1/6] libfdt: overlay: Pass pointer to blob pointer in fdt_overlay_apply()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



On Fri, Mar 16, 2018 at 06:36:55PM +0530, Srivatsa Vaddagiri wrote:
> Change input argument for fdt_overlay_apply() to accept pointer to a blob
> pointer rather than just a pointer to blob. Merging overlay blobs together
> requires both base and overlay blobs to be resized by an amount that can't be
> exactly determined ahead of time. overlay blob, for example, will need to be
> resized for change in fragment names and base blob will need to be resized to
> possibly include a __local_fixups__ section.

As noted on 0/6 this changes an existing interface, which isn't
ok. Use a new function instead.

> 
> Since resize via realloc could relocate buffers in virtual memory, caller of
> fdt_overlay_apply() will need to know where the (modified) blobs are located.

Plus, this doesn't look like an acceptable interface for libfdt
anyway.  libfdt does not rely on an allocator, since it's supposed to
be usable in highly constrained environments which don't provide one.
I don't want to change that.


> Signed-off-by: Srivatsa Vaddagiri <vatsa@xxxxxxxxxxxxxx>
> ---
>  fdtoverlay.c         |  3 ++-
>  libfdt/fdt_overlay.c | 24 ++++++++++++------------
>  libfdt/libfdt.h      | 11 ++++++++---
>  3 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/fdtoverlay.c b/fdtoverlay.c
> index 62a942d..8aed173 100644
> --- a/fdtoverlay.c
> +++ b/fdtoverlay.c
> @@ -100,7 +100,8 @@ static int do_fdtoverlay(const char *input_filename,
>  
>  	/* apply the overlays in sequence */
>  	for (i = 0; i < argc; i++) {
> -		ret = fdt_overlay_apply(blob, ovblob[i]);
> +		ret = fdt_overlay_apply((void **)&blob,
> +						(void **)&ovblob[i]);
>  		if (ret) {
>  			fprintf(stderr, "\nFailed to apply %s (%d)\n",
>  					argv[i], ret);
> diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
> index bf75388..64386dd 100644
> --- a/libfdt/fdt_overlay.c
> +++ b/libfdt/fdt_overlay.c
> @@ -861,38 +861,38 @@ static int overlay_symbol_update(void *fdt, void *fdto)
>  	return 0;
>  }
>  
> -int fdt_overlay_apply(void *fdt, void *fdto)
> +int fdt_overlay_apply(void **fdt, void **fdto)
>  {
> -	uint32_t delta = fdt_get_max_phandle(fdt);
> +	uint32_t delta = fdt_get_max_phandle(*fdt);
>  	int ret;
>  
> -	FDT_CHECK_HEADER(fdt);
> -	FDT_CHECK_HEADER(fdto);
> +	FDT_CHECK_HEADER(*fdt);
> +	FDT_CHECK_HEADER(*fdto);
>  
> -	ret = overlay_adjust_local_phandles(fdto, delta);
> +	ret = overlay_adjust_local_phandles(*fdto, delta);
>  	if (ret)
>  		goto err;
>  
> -	ret = overlay_update_local_references(fdto, delta);
> +	ret = overlay_update_local_references(*fdto, delta);
>  	if (ret)
>  		goto err;
>  
> -	ret = overlay_fixup_phandles(fdt, fdto);
> +	ret = overlay_fixup_phandles(*fdt, *fdto);
>  	if (ret)
>  		goto err;
>  
> -	ret = overlay_merge(fdt, fdto);
> +	ret = overlay_merge(*fdt, *fdto);
>  	if (ret)
>  		goto err;
>  
> -	ret = overlay_symbol_update(fdt, fdto);
> +	ret = overlay_symbol_update(*fdt, *fdto);
>  	if (ret)
>  		goto err;
>  
>  	/*
>  	 * The overlay has been damaged, erase its magic.
>  	 */
> -	fdt_set_magic(fdto, ~0);
> +	fdt_set_magic(*fdto, ~0);
>  
>  	return 0;
>  
> @@ -900,13 +900,13 @@ err:
>  	/*
>  	 * The overlay might have been damaged, erase its magic.
>  	 */
> -	fdt_set_magic(fdto, ~0);
> +	fdt_set_magic(*fdto, ~0);
>  
>  	/*
>  	 * The base device tree might have been damaged, erase its
>  	 * magic.
>  	 */
> -	fdt_set_magic(fdt, ~0);
> +	fdt_set_magic(*fdt, ~0);
>  
>  	return ret;
>  }
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index c8c00fa..5dd63ef 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -1864,8 +1864,8 @@ int fdt_del_node(void *fdt, int nodeoffset);
>  
>  /**
>   * fdt_overlay_apply - Applies a DT overlay on a base DT
> - * @fdt: pointer to the base device tree blob
> - * @fdto: pointer to the device tree overlay blob
> + * @fdt: pointer to base device tree blob pointer
> + * @fdto: pointer to device tree overlay blob pointer
>   *
>   * fdt_overlay_apply() will apply the given device tree overlay on the
>   * given base device tree.
> @@ -1873,6 +1873,10 @@ int fdt_del_node(void *fdt, int nodeoffset);
>   * Expect the base device tree to be modified, even if the function
>   * returns an error.
>   *
> + * Memory pointed to by @fdt and/or @fdto may be resized via mrealloc()
> + * and hence could point to different memory area upon return of this
> + * function.
> + *
>   * returns:
>   *	0, on success
>   *	-FDT_ERR_NOSPACE, there's not enough space in the base device tree
> @@ -1891,7 +1895,8 @@ int fdt_del_node(void *fdt, int nodeoffset);
>   *	-FDT_ERR_BADSTATE,
>   *	-FDT_ERR_TRUNCATED, standard meanings
>   */
> -int fdt_overlay_apply(void *fdt, void *fdto);
> +
> +int fdt_overlay_apply(void **fdt, void **fdto);
>  
>  /**********************************************************************/
>  /* Debugging / informational functions                                */

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


[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux