[v6,2/2] tools/nolibc: fix up size inflate regression

As reported and suggested by Willy, the inline __sysret() helper
introduces three types of conversions and increases the size:

(1) the "unsigned long" argument to __sysret() forces a sign extension
from all sys_* functions that used to return 'int'

(2) the comparison with the error range now has to be performed on a
'unsigned long' instead of an 'int'

(3) the return value from __sysret() is a 'long' (note, a signed long)
which then has to be turned back to an 'int' before being returned by the
caller to satisfy the caller's prototype.

To fix up this, firstly, let's use macro instead of inline function to
preserves the input type and avoids these useless conversions (1), (3).

Secondly, since all of the sys_* functions have been converted to return
integer, now, it is able to remove comparison to a 'unsigned long'
-MAX_ERRNO (2) and restore the simple sign comparison as before.

Suggested-by: Willy Tarreau <w@1wt.eu>
Link: https://lore.kernel.org/lkml/20230806095846.GB10627@1wt.eu/
Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
---
 tools/include/nolibc/sys.h | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

On Sat, Aug 12, 2023 at 05:51:53AM +0800, Zhangjin Wu wrote:
> As reported and suggested by Willy, the inline __sysret() helper
> introduces three types of conversions and increases the size:
> 
> (1) the "unsigned long" argument to __sysret() forces a sign extension
> from all sys_* functions that used to return 'int'
> 
> (2) the comparison with the error range now has to be performed on a
> 'unsigned long' instead of an 'int'
> 
> (3) the return value from __sysret() is a 'long' (note, a signed long)
> which then has to be turned back to an 'int' before being returned by the
> caller to satisfy the caller's prototype.
> 
> To fix up this, firstly, let's use macro instead of inline function to
> preserves the input type and avoids these useless conversions (1), (3).
> 
> Secondly, since all of the sys_* functions have been converted to return
> integer, now, it is able to remove comparison to a 'unsigned long'
> -MAX_ERRNO (2) and restore the simple sign comparison as before.
> 
(...)
> +/* Syscall return helper, set errno as -ret when ret < 0 */
> +#define __sysret(arg)                        \
> +({                                           \
> +	__typeof__(arg) __ret = (arg);       \
> +	if (__ret < 0) {                     \
> +		SET_ERRNO(-__ret);           \
> +		__ret = -1L;                 \
> +	}                                    \
> +	__ret;                               \
> +})

Except that this now breaks brk(), mmap() and sbrk() by taking any value
with MSB set as an error. Also you've re-introduced the problem you've
faced with const. See my simplification in the other thread by using "?:"
which does avoids any assignment.

Let's just roll brk(), mmap() and sbrk() to their original, working,
definition:

 static __attribute__((unused))
 void *mmap(void *addr, size_t length, int prot, int flags, int fd, off_t offset)
 {
        void *ret = sys_mmap(addr, length, prot, flags, fd, offset);
 
        if ((unsigned long)ret >= -MAX_ERRNO) {
                SET_ERRNO(-(long)ret);
                ret = MAP_FAILED;
        }
        return ret;
 }

And we're done, you can then keep the simplified __sysret() macro for all
other call places.

Willy