Re: [PATCH v2] xen/dt: Remove loop in dt_read_number()

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 17 Jun 2025, Alejandro Vallejo wrote:
> The DT spec declares only two number types for a property: u32 and u64,
> as per Table 2.3 in Section 2.2.4. Remove unbounded loop and replace
> with a switch statement. Default to a size of 1 cell in the nonsensical
> size case, with a warning printed on the Xen console.
> 
> Suggested-by: Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Alejandro Vallejo <agarciav@xxxxxxx>
> ---
> v2:
>   * Added missing `break` on the `case 2:` branch and added 
> ASSERT_UNREACHABLE() to the deafult path
> ---
>  xen/include/xen/device_tree.h | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index 75017e4266..2ec668b94a 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -261,10 +261,21 @@ void intc_dt_preinit(void);
>  /* Helper to read a big number; size is in cells (not bytes) */
>  static inline u64 dt_read_number(const __be32 *cell, int size)
>  {
> -    u64 r = 0;
> +    u64 r = be32_to_cpu(*cell);
> +
> +    switch ( size )
> +    {
> +    case 1:
> +        break;
> +    case 2:
> +        r = (r << 32) | be32_to_cpu(cell[1]);
> +        break;
> +    default:
> +        // Nonsensical size. default to 1.

NIT: comment style is /* comment */

> +        printk(XENLOG_WARNING "dt_read_number(%d) bad size\n", size);
> +        ASSERT_UNREACHABLE();

You need a break statement even after ASSERT_UNREACHABLE() for MISRA R16.3


> +    };
>  
> -    while ( size-- )
> -        r = (r << 32) | be32_to_cpu(*(cell++));
>      return r;
>  }
>  
> 
> base-commit: 14c57887f36937c1deb9eeca852c3a7595d2d0b8
> -- 
> 2.43.0
>