This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH 1/6] Add INLINE_SYSCALL_RETURN/INLINE_SYSCALL_ERROR_RETURN
- From: Joseph Myers <joseph at codesourcery dot com>
- To: "H.J. Lu" <hjl dot tools at gmail 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 16:34:11 +0000
- 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> <CAMe9rOrLUBih4UOrn=bb9uwi29y1FzzYUTwxHxLwRQHzYjZDZQ at mail dot gmail dot com> <alpine dot DEB dot 2 dot 10 dot 1508211612030 dot 2039 at digraph dot polyomino dot org dot uk> <CAMe9rOrrU-=AmYKHSaN2j6RmXJ3pP2ftSYewaSUWqBtnQVZg9Q at mail dot gmail dot com>
On Fri, 21 Aug 2015, H.J. Lu wrote:
> > I think the fix process should not happen on master, but a separate
> > branch, with the changes being reverted until they have consensus.
> > This isn't a matter of fixing isolated bugs that a reviewer missed - if it
> > were just that, then fixing on master would be fine. It's a matter of
> > there being no consensus on the basic design of the changes or the overall
> > shape of the new interfaces being introduced (and no detailed review of
> > the changes either).
> I did my work on a branch, which lead us here. I can take out
What got us here was committing the change to master when there was no
consensus for it (no-one had actually reviewed the change carefully,
looking at both the design of the interfaces involved and the specific
changes to use them).
That isn't appropriate even for x86-specific changes (those still need
consensus). It's especially inappropriate for a general change to core
architecture-independent code (Linux syscall handling), with design issues
> the libc_hidden_def change in sysdeps/unix/sysv/linux/sched_getaffinity.c
> which makes my change pure mechanical.
The change involves introducing new interfaces. The design of those
interfaces is not a mechanical matter - it's something that needs careful
consideration before the interfaces go on master, which simply hasn't
happened. Even with the removal of the last argument to
INLINE_SYSCALL_ERROR_RETURN, I think the fact that it expects a negated
errno argument and loads of callers have such a negation within the call
indicates a defect in the design - a macro expecting a negated value
should be used *only* in the cases where that's a direct pass through of
the syscall return value in the caller.
Joseph S. Myers