Re: [PATCH] of: never fixup internal live tree

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 03.08.23 10:51, Sascha Hauer wrote:
> Calling of_fix_tree() on the internal live tree has undesired side
> effects, so refrain from doing so.
> 
> One of the side effects is that some parts of barebox store pointers
> to properties in the live tree which might become invalid when during
> of_fixup these properties are deleted or updated.
> 
> The other is that it's unexpected that the live tree is modified
> after a dry run boot with the live tree.
> 
> This changes of_get_fixed_tree() to always call of_fix_tree() on
> a copy of the device tree and not on the device tree itself. To
> emphasize this make the device tree argument to of_get_fixed_tree()
> const.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  common/oftree.c | 19 ++++++++++---------
>  include/of.h    |  2 +-
>  2 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/common/oftree.c b/common/oftree.c
> index 3e85070d11..51eebd36bd 100644
> --- a/common/oftree.c
> +++ b/common/oftree.c
> @@ -416,26 +416,27 @@ void of_fix_tree(struct device_node *node)
>   * It increases the size of the tree and applies the registered
>   * fixups.
>   */
> -struct fdt_header *of_get_fixed_tree(struct device_node *node)
> +struct fdt_header *of_get_fixed_tree(const struct device_node *node)
>  {
>  	struct fdt_header *fdt = NULL;
> -	struct device_node *freenp = NULL;
> +	struct device_node *np;
>  
>  	if (!node) {
>  		node = of_get_root_node();
>  		if (!node)
>  			return NULL;
> -
> -		freenp = node = of_dup(node);
> -		if (!node)
> -			return NULL;
>  	}
>  
> -	of_fix_tree(node);
> +	np = of_dup(node);
> +
> +	if (!np)
> +		return NULL;
> +
> +	of_fix_tree(np);
>  
> -	fdt = of_flatten_dtb(node);
> +	fdt = of_flatten_dtb(np);
>  
> -	of_delete_node(freenp);
> +	of_delete_node(np);
>  
>  	return fdt;
>  }
> diff --git a/include/of.h b/include/of.h
> index 92a15f5c4a..0deb327971 100644
> --- a/include/of.h
> +++ b/include/of.h
> @@ -75,7 +75,7 @@ int of_add_initrd(struct device_node *root, resource_size_t start,
>  
>  struct fdt_header *fdt_get_tree(void);
>  
> -struct fdt_header *of_get_fixed_tree(struct device_node *node);
> +struct fdt_header *of_get_fixed_tree(const struct device_node *node);
>  
>  /* Helper to read a big number; size is in cells (not bytes) */
>  static inline u64 of_read_number(const __be32 *cell, int size)

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |