Re: [PATCH] xen/arm: fdt fix duplicated ternary operator, shift operations

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Sat, 23 Apr 2022, Paran Lee wrote:
> It doesn't seem necessary to do duplicate ternary operation and calculation
> of order shift using fdt32_to_cpu macro.
> 
> Signed-off-by: Paran Lee <p4ranlee@xxxxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
>  xen/arch/arm/bootfdt.c  | 12 ++++++++++--
>  xen/common/libfdt/fdt.c | 10 +++++-----
>  2 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> index e318ef9603..e5b885a7f2 100644
> --- a/xen/arch/arm/bootfdt.c
> +++ b/xen/arch/arm/bootfdt.c
> @@ -159,8 +159,16 @@ int __init device_tree_for_each_node(const void *fdt, 
> int node,
>              continue;
>          }
>  
> -        as = depth > 0 ? address_cells[depth-1] : 
> DT_ROOT_NODE_ADDR_CELLS_DEFAULT;
> -        ss = depth > 0 ? size_cells[depth-1] : 
> DT_ROOT_NODE_SIZE_CELLS_DEFAULT;
> +        if ( depth > 0 )
> +        {
> +            as = address_cells[depth-1];
> +            ss = size_cells[depth-1];
> +        }
> +        else
> +        {
> +            as = DT_ROOT_NODE_ADDR_CELLS_DEFAULT;
> +            ss = DT_ROOT_NODE_SIZE_CELLS_DEFAULT;
> +        }
>  
>          address_cells[depth] = device_tree_get_u32(fdt, node,
>                                                     "#address-cells", as);
> diff --git a/xen/common/libfdt/fdt.c b/xen/common/libfdt/fdt.c
> index 9fe7cf4b74..a507169d29 100644
> --- a/xen/common/libfdt/fdt.c
> +++ b/xen/common/libfdt/fdt.c
> @@ -165,7 +165,7 @@ const void *fdt_offset_ptr(const void *fdt, int offset, 
> unsigned int len)
>  uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
>  {
>       const fdt32_t *tagp, *lenp;
> -     uint32_t tag;
> +     uint32_t tag, len;
>       int offset = startoffset;
>       const char *p;
>  
> @@ -192,11 +192,11 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, 
> int *nextoffset)
>               if (!can_assume(VALID_DTB) && !lenp)
>                       return FDT_END; /* premature end */
>               /* skip-name offset, length and value */
> -             offset += sizeof(struct fdt_property) - FDT_TAGSIZE
> -                     + fdt32_to_cpu(*lenp);
> +             len = fdt32_to_cpu(*lenp);
> +             offset += sizeof(struct fdt_property) - FDT_TAGSIZE + len;
>               if (!can_assume(LATEST) &&
> -                 fdt_version(fdt) < 0x10 && fdt32_to_cpu(*lenp) >= 8 &&
> -                 ((offset - fdt32_to_cpu(*lenp)) % 8) != 0)
> +                 fdt_version(fdt) < 0x10 && len >= 8 &&
> +                 ((offset - len) % 8) != 0)
>                       offset += 4;
>               break;
>  
> -- 
> 2.25.1
>