[PATCH] include: sbi: fix swap errors with newer gcc -Werror=sequence-point

[Anup Patel]

On Fri, Jul 4, 2025 at 5:59 PM Ben Dooks <ben.dooks at codethink.co.uk> wrote:
>
> The BSWAPxx() macros are now throwing the following warnings with
> newer gcc versions. This is due to throwing an argument in that may
> be evaluated more than one (I think) and therefore things like the
> example below should be avoided.
>
> Fix by making a set of BSWAPxx() wrappers which specifically onlt
> evaluate 'x' once.
>
> In file included lib/sbi/sbi_mpxy.c:21:
> lib/sbi/sbi_mpxy.c: In function ‘sbi_mpxy_write_attrs’:
> ib/sbi/sbi_mpxy.c:632:63: error: operation on ‘mem_idx’ may be undefined [-Werror=sequence-point]
>   632 |                         attr_val = le32_to_cpu(mem_ptr[mem_idx++]);
>       |                                                        ~~~~~~~^~
>
> Signed-off-by: Ben Dooks <ben.dooks at codethink.co.uk>

I have taken care of Rahul's comment at the time
of merging this patch.

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup

> ---
>  include/sbi/sbi_byteorder.h | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/include/sbi/sbi_byteorder.h b/include/sbi/sbi_byteorder.h
> index 2b4981e..ed7cad1 100644
> --- a/include/sbi/sbi_byteorder.h
> +++ b/include/sbi/sbi_byteorder.h
> @@ -14,13 +14,13 @@
>  # define _conv_cast(type, val) ((type)(val))
>  #endif
>
> -#define BSWAP16(x)     ((((x) & 0x00ff) << 8) | \
> +#define __BSWAP16(x)   ((((x) & 0x00ff) << 8) | \
>                          (((x) & 0xff00) >> 8))
> -#define BSWAP32(x)     ((((x) & 0x000000ff) << 24) | \
> +#define __BSWAP32(x)   ((((x) & 0x000000ff) << 24) | \
>                          (((x) & 0x0000ff00) << 8) | \
>                          (((x) & 0x00ff0000) >> 8) | \
>                          (((x) & 0xff000000) >> 24))
> -#define BSWAP64(x)     ((((x) & 0x00000000000000ffULL) << 56) | \
> +#define __BSWAP64(x)   ((((x) & 0x00000000000000ffULL) << 56) | \
>                          (((x) & 0x000000000000ff00ULL) << 40) | \
>                          (((x) & 0x0000000000ff0000ULL) << 24) | \
>                          (((x) & 0x00000000ff000000ULL) << 8) | \
> @@ -29,6 +29,10 @@
>                          (((x) & 0x00ff000000000000ULL) >> 40) | \
>                          (((x) & 0xff00000000000000ULL) >> 56))
>
> +#define BSWAP64(x) ({ uint64_t _sv = (x); __BSWAP64(_sv); })
> +#define BSWAP32(x) ({ uint32_t _sv = (x); __BSWAP32(_sv); })
> +#define BSWAP16(x) ({ uint16_t _sv = (x); __BSWAP16(_sv); })
> +
>  #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__  /* CPU(little-endian) */
>  #define cpu_to_be16(x)         _conv_cast(uint16_t, BSWAP16(x))
>  #define cpu_to_be32(x)         _conv_cast(uint32_t, BSWAP32(x))
> --
> 2.37.2.352.g3c44437643
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi