Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   David Laight <David.Laight@ACULAB.COM>
To:     'Zhangjin Wu' <falcon@tinylab.org>, "w@1wt.eu" <w@1wt.eu>
CC:     "arnd@arndb.de" <arnd@arndb.de>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-kselftest@vger.kernel.org" <linux-kselftest@vger.kernel.org>,
        "thomas@t-8ch.de" <thomas@t-8ch.de>
Subject: RE: [PATCH v5] tools/nolibc: fix up size inflate regression
Thread-Topic: [PATCH v5] tools/nolibc: fix up size inflate regression
Thread-Index: AQHZzpwIMKbqPe7hTkqs7/1zyBEHW6/pnUhg
Date:   Mon, 14 Aug 2023 11:15:51 +0000
Message-ID: <6fef903020954515abdcee7261918903@AcuMS.aculab.com>
References: <20230814082224.GA16761@1wt.eu>
 <20230814104226.7094-1-falcon@tinylab.org>
In-Reply-To: <20230814104226.7094-1-falcon@tinylab.org>
Accept-Language: en-GB, en-US
MIME-Version: 1.0
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Precedence: bulk

From: Zhangjin Wu
> Sent: 14 August 2023 11:42
...
> [...]
> > > > Sure it's not pretty, and I'd rather just go back to SET_ERRNO() to be
> > > > honest, because we're there just because of the temptation to remove
> > > > lines that were not causing any difficulties :-/
> > > >
> > > > I think we can do something in-between and deal only with signed returns,
> > > > and explicitly place the test for MAX_ERRNO on the two unsigned ones
> > > > (brk and mmap). It should look approximately like this:
> > > >
> > > >  #define __sysret(arg)                                                \
> > > >  ({                                                                   \
> > > >  	__typeof__(arg) __sysret_arg = (arg);                           \
> > > >  	(__sysret_arg < 0) ? ({           /* error ? */                 \
> > > >  		SET_ERRNO(-__sysret_arg); /* yes: errno != -ret */      \
> > > >  		((__typeof__(arg)) -1);   /*      return -1 */          \

I'm pretty sure you don't need the explicit cast.
(It would be needed for a pointer type.)
Can you use __arg < ? SET_ERRNO(-__arg), -1 : __arg

Thinking, maybe it should be:

#define __sysret(syscall_fn_args)
({
	__typeof__(syscall_fn_args) __rval = syscall_fn_args;
	__rval >= 0 ? __rval : SET_ERRNO(-__rval), -1;
})

Since, IIRC, the usage is return __sysret(sycall_fn(args));

I'm not sure how public SET_ERRO() is.
But it could include the negate have the value of -1 cast to its argument type?
I think:
	error = -(int)(long)(arg + 0u);
will avoid any sign extension - the (int) might not even be needed.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)