On Sun, Aug 06, 2023 at 09:19:21PM +0800, Zhangjin Wu wrote:
> As reported and suggested by Willy, the inline __sysret() helper
> introduces three types of conversions and increases the size:
Thanks Zhangjin. A few things:
> --- a/tools/include/nolibc/compiler.h
> +++ b/tools/include/nolibc/compiler.h
> @@ -22,4 +22,13 @@
> # define __no_stack_protector __attribute__((__optimize__("-fno-stack-protector")))
> #endif /* defined(__has_attribute) */
>
> +/*
> + * from include/linux/compiler.h
> + *
> + * Whether 'type' is a signed type or an unsigned type. Supports scalar types,
> + * bool and also pointer types.
> + */
> +#define is_signed_type(type) (((type)(-1)) < (type)1)
> +#define is_unsigned_type(type) (!is_signed_type(type))
These names may conflict with application's local definitions. And since
there's a single call place we should probably just inline it instead.
> +#define __sysret(arg) \
> +({ \
> + __typeof__(arg) __sysret_arg = (arg); \
> + if (is_signed_type(__typeof__(arg))) { \
> + if (__sysret_arg < 0) { \
> + SET_ERRNO(-(int)__sysret_arg); \
> + __sysret_arg = -1L; \
> + } \
> + } else { \
> + if ((unsigned long)__sysret_arg >= (unsigned long)-MAX_ERRNO) { \
> + SET_ERRNO(-(int)__sysret_arg); \
> + __sysret_arg = -1L; \
> + } \
> + } \
> + __sysret_arg; \
> +})
I also found during my tests that this one can return a build error if
the arg has "const" in its type, due to the error assignment. We need
to think about reworking it as a ternary evaluation, it will be more
reliable even if less readable. But let's not change this now, I'm on
the changelog already.
> /* Functions in this file only describe syscalls. They're declared static so
> * that the compiler usually decides to inline them while still being allowed
> @@ -94,7 +101,7 @@ void *sbrk(intptr_t inc)
> if (ret && sys_brk(ret + inc) == ret + inc)
> return ret + inc;
>
> - return (void *)__sysret(-ENOMEM);
> + return (void *)__sysret((unsigned long)-ENOMEM);
I noticed that one as well during my tests, but if we're purist, we're
supposed to use (void*) for the cast and not (unsigned long), and it
should allow to get rid of the outer cast.
Regards,
Willy
