Re: [PATCH 1/6] Add INLINE_SYSCALL_RETURN/INLINE_SYSCALL_ERROR_RETURN

[Joseph Myers <joseph at codesourcery 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 
involved.

> 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
joseph@codesourcery.com