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