On Wed, Jul 9, 2025 at 12:53 AM Philippe Mathieu-Daudé
<phi...@linaro.org> wrote:
>
> Check endianness at runtime to remove the target-specific
> TARGET_BIG_ENDIAN definition. Use cpu_to_[be,le]XX() from
> "qemu/bswap.h" instead of tswapXX() from "exec/tswap.h".
>
> Suggested-by: Richard Henderson <richard.hender...@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> ---
> include/gdbstub/helpers.h | 48 +++++++++++++++++++++++----------------
> 1 file changed, 29 insertions(+), 19 deletions(-)
>
> diff --git a/include/gdbstub/helpers.h b/include/gdbstub/helpers.h
> index 6f7cc48adcb..b685afac436 100644
> --- a/include/gdbstub/helpers.h
> +++ b/include/gdbstub/helpers.h
> @@ -16,7 +16,8 @@
> #error "gdbstub helpers should only be included by target specific code"
> #endif
>
> -#include "exec/tswap.h"
> +#include "qemu/bswap.h"
> +#include "qemu/target-info.h"
> #include "cpu-param.h"
>
> /*
> @@ -33,40 +34,49 @@ static inline int gdb_get_reg8(GByteArray *buf, uint8_t
> val)
>
> static inline int gdb_get_reg16(GByteArray *buf, uint16_t val)
> {
> - uint16_t to_word = tswap16(val);
> - g_byte_array_append(buf, (uint8_t *) &to_word, 2);
> + if (target_big_endian()) {
> + cpu_to_be16s(&val);
Unrelated, but that 's' in 'cpu_to_be16s' is non-obvious, I always
think it's about signedness which is unrelated to endianness. I guess
it stands for "specified". It's more intuitive to call by-value
functions 'into' and by-ref 'to' maybe? e.g. `uint16_t
cpu_into_le16(uint16_t v);` and `void cpu_to_le16(uint16_t *v);`.
> + } else {
> + cpu_to_le16s(&val);
> + }
> + g_byte_array_append(buf, (uint8_t *) &val, 2);
> return 2;
> }
>
> static inline int gdb_get_reg32(GByteArray *buf, uint32_t val)
> {
> - uint32_t to_long = tswap32(val);
> - g_byte_array_append(buf, (uint8_t *) &to_long, 4);
> + if (target_big_endian()) {
> + cpu_to_be32s(&val);
> + } else {
> + cpu_to_le32s(&val);
> + }
> + g_byte_array_append(buf, (uint8_t *) &val, 4);
> return 4;
> }
>
> static inline int gdb_get_reg64(GByteArray *buf, uint64_t val)
> {
> - uint64_t to_quad = tswap64(val);
> - g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
> + if (target_big_endian()) {
> + cpu_to_be64s(&val);
> + } else {
> + cpu_to_le64s(&val);
> + }
> + g_byte_array_append(buf, (uint8_t *) &val, 8);
> return 8;
> }
>
> static inline int gdb_get_reg128(GByteArray *buf, uint64_t val_hi,
> uint64_t val_lo)
> {
> - uint64_t to_quad;
> -#if TARGET_BIG_ENDIAN
> - to_quad = tswap64(val_hi);
> - g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
> - to_quad = tswap64(val_lo);
> - g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
> -#else
> - to_quad = tswap64(val_lo);
> - g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
> - to_quad = tswap64(val_hi);
> - g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
> -#endif
> + uint64_t tmp[2];
> + if (target_big_endian()) {
> + tmp[0] = cpu_to_be64(val_hi);
> + tmp[1] = cpu_to_be64(val_lo);
> + } else {
> + tmp[0] = cpu_to_le64(val_lo);
> + tmp[1] = cpu_to_le64(val_hi);
> + }
> + g_byte_array_append(buf, (uint8_t *)&tmp, 16);
> return 16;
> }
>
> --
> 2.49.0
>
Reviewed-by: Manos Pitsidianakis <manos.pitsidiana...@linaro.org>
