This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 1/6] Add INLINE_SYSCALL_RETURN/INLINE_SYSCALL_ERROR_RETURN
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Joseph Myers <joseph at codesourcery dot com>
- Cc: Andreas Schwab <schwab at linux-m68k dot org>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Fri, 21 Aug 2015 10:40:16 -0700
- Subject: Re: [PATCH 1/6] Add INLINE_SYSCALL_RETURN/INLINE_SYSCALL_ERROR_RETURN
- Authentication-results: sourceware.org; auth=none
- References: <20150814120014 dot GA28610 at gmail dot com> <87oaiavy2c dot fsf at igel dot home> <CAMe9rOoP9GPP+i6xbAXwHffwr+KOKNhV=FsJ5sx=G2bM_1SE+g at mail dot gmail dot com> <87oai9vkg7 dot fsf at igel dot home> <CAMe9rOqPmsPPR7RPbdhMKh-cRxK_J2dphmECdvq0QHtfnLgz0w at mail dot gmail dot com> <87k2sxvjh8 dot fsf at igel dot home> <CAMe9rOrbhzbxg2ed3w0i7zE7=KM1q37DtR1vAY_Lk=rxsJ_8mA at mail dot gmail dot com> <87d1ypvixm dot fsf at igel dot home> <CAMe9rOqEpVE3X-drZp6W6UESV0CCAtY3Qew+SPOt1b7Y9BNwfA at mail dot gmail dot com> <878u9dvh61 dot fsf at igel dot home> <CAMe9rOqNhBrGkJCkp5xT8DebN68qzoZciWtSZzGhYo1Msyv3TQ at mail dot gmail dot com> <CAMe9rOqZ=4B698SouQm=fTLaZvsqStQRYyMMhjPNCKMbjK1Xmw at mail dot gmail dot com> <CAMe9rOqNodDbB2mi4SU6agR=-qK0yfRHyFq7BiwBeWre-gJ1tw at mail dot gmail dot com> <alpine dot DEB dot 2 dot 10 dot 1508211502380 dot 2039 at digraph dot polyomino dot org dot uk> <878u94ws4b dot fsf at igel dot home> <alpine dot DEB dot 2 dot 10 dot 1508211707230 dot 2039 at digraph dot polyomino dot org dot uk>
On Fri, Aug 21, 2015 at 10:17 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Fri, 21 Aug 2015, Andreas Schwab wrote:
>
>> Joseph Myers <joseph@codesourcery.com> writes:
>>
>> > I suspect the patch would also better be split up into the most mechanical
>> > changes that don't change generated code at all, and the less obvious
>> > pieces needing more careful review.
>>
>> Especially nothing should be committed until the whole series is aggreed
>> upon.
>
> I don't think that's a good general principle for patch series - a patch
> may well be good on its own merits independent of subsequent patches based
> on it. But in the case of patch 1/6 here, I agree it only makes sense if
> subsequent patches are agreed.
>
> Specifically, I don't see any need for either of the new macros. So I
> think the whole basis of this first patch is flawed, since its
> justification is for use in the subsequent patches.
>
> Instead of having two new macros, I think the i386 INLINE_SYSCALL could be
> made to use __syscall_error by making its contents look like
> (appropriately formatted):
>
> ({
> unsigned int resultvar = INTERNAL_SYSCALL (name, , nr, args);
> __glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (resultvar, )) ?
> __syscall_error (resultvar) : resultvar;
> })
The return type of __syscall_error will be wrong in this case.
That is the ONLY reason why I added:
/* Similar to INLINE_SYSCALL, but with return type. It should only be
used as function return value. */
#ifndef INLINE_SYSCALL_RETURN
#define INLINE_SYSCALL_RETURN(name, nr, type, args...) \
INLINE_SYSCALL (name, nr, args)
#endif
so that I can do
({
unsigned int resultvar = INTERNAL_SYSCALL (name, , nr, args);
__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (resultvar, )) ?
(type) __syscall_error (resultvar) : resultvar;
})
But I don't want to add the new argument to INLINE_SYSCALL
when there is no benefit to call __syscall_error.
--
H.J.