On Tue, Aug 8, 2023 at 3:03 PM Richard Henderson <
richard.hender...@linaro.org> wrote:
> On 8/7/23 23:07, Karim Taha wrote:
> > From: Warner Losh <i...@bsdimp.com>
> >
> > Use __builtin_choose_expr to avoid type promotion from ?:
> > in __put_user_e and __get_user_e macros.
> > Copied from linux-user/qemu.h, originally by Blue Swirl.
> >
> > Signed-off-by: Warner Losh <i...@bsdimp.com>
> > Signed-off-by: Karim Taha <kariem.taha...@gmail.com>
> > ---
> > bsd-user/qemu.h | 81 ++++++++++++++++++++---------------------------
> > bsd-user/signal.c | 5 +--
> > 2 files changed, 35 insertions(+), 51 deletions(-)
> >
> > diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> > index dfdfa8dd67..c41ebfe937 100644
> > --- a/bsd-user/qemu.h
> > +++ b/bsd-user/qemu.h
> > @@ -307,50 +307,37 @@ static inline bool access_ok(int type, abi_ulong
> addr, abi_ulong size)
> > #define PRAGMA_REENABLE_PACKED_WARNING
> > #endif
> >
> > -#define __put_user(x, hptr)\
> > -({\
> > - int size = sizeof(*hptr);\
> > - switch (size) {\
> > - case 1:\
> > - *(uint8_t *)(hptr) = (uint8_t)(typeof(*hptr))(x);\
> > - break;\
> > - case 2:\
> > - *(uint16_t *)(hptr) = tswap16((typeof(*hptr))(x));\
> > - break;\
> > - case 4:\
> > - *(uint32_t *)(hptr) = tswap32((typeof(*hptr))(x));\
> > - break;\
> > - case 8:\
> > - *(uint64_t *)(hptr) = tswap64((typeof(*hptr))(x));\
> > - break;\
> > - default:\
> > - abort();\
> > - } \
> > - 0;\
> > -})
> > +#define __put_user_e(x, hptr, e)
> \
> > + do {
> \
> > + PRAGMA_DISABLE_PACKED_WARNING;
> \
> > + (__builtin_choose_expr(sizeof(*(hptr)) == 1, stb_p,
> \
> > + __builtin_choose_expr(sizeof(*(hptr)) == 2, stw_##e##_p,
> \
> > + __builtin_choose_expr(sizeof(*(hptr)) == 4, stl_##e##_p,
> \
> > + __builtin_choose_expr(sizeof(*(hptr)) == 8, stq_##e##_p,
> abort)))) \
> > + ((hptr), (x)), (void)0);
> \
> > + PRAGMA_REENABLE_PACKED_WARNING;
> \
> > + } while (0)
> > +
> > +#define __get_user_e(x, hptr, e)
> \
> > + do {
> \
> > + PRAGMA_DISABLE_PACKED_WARNING;
> \
> > + ((x) = (typeof(*hptr))(
> \
> > + __builtin_choose_expr(sizeof(*(hptr)) == 1, ldub_p,
> \
> > + __builtin_choose_expr(sizeof(*(hptr)) == 2, lduw_##e##_p,
> \
> > + __builtin_choose_expr(sizeof(*(hptr)) == 4, ldl_##e##_p,
> \
> > + __builtin_choose_expr(sizeof(*(hptr)) == 8, ldq_##e##_p,
> abort)))) \
> > + (hptr)), (void)0);
> \
> > + PRAGMA_REENABLE_PACKED_WARNING;
> \
> > + } while (0)
>
> Hmm. I guess this works. The typeof cast in __get_user_e being required
> when sizeof(x) >
> sizeof(*hptr) in order to get the correct extension.
>
This code was copied 100% from the current linux-user :) It was also copied
before I needed
to do _Generic for a different project for something as well, so I didn't
think to switch over
from the old-school, gcc-specific hack to the modern clean form.
> Is it clearer with _Generic?
>
> (x) = _Generic(*(hptr),
> int8_t: *(int8_t *)(hptr),
> uint8_t: *(uint8_t *)(hptr),
> int16_t: (int16_t)lduw_##e##_p(hptr),
> uint16_t: lduw_##e##_p(hptr),
> int32_t: (int32_t)ldl_##e##_p(hptr),
> uint32_t: (uint32_t)ldl_##e##_p(hptr),
> int64_t: (int64_t)ldq_##e##_p(hptr),
> uint64_t: ldq_##e##_p(hptr));
>
> In particular I believe the error message will be much prettier.
>
Indeed. That looks cleaner. I'll see if I can test that against the latest
bsd-user upstream.
Warner
> Either way,
> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
>
>
> r~
>
