Re: [PATCH 2/6] Optimize i386 syscall inlining

[Joseph Myers <joseph at codesourcery dot com>]

On Fri, 21 Aug 2015, H.J. Lu wrote:

> On Fri, Aug 21, 2015 at 5:35 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> > You define INLINE_SYSCALL_ERROR_RETURN to ignore the specified return
> > value.  There is nothing in the comments on the default version of that
> > macro in sysdeps/unix/sysdep.h to say that it is OK to ignore the return
> > value.  If the return value must always be -1, I think the macro should
> > take one fewer argument rather than having specified semantics that the
> > last argument must always be -1, or that the macro may return -1 and
> > ignore the last argument.
> 
> __syscall_error is for tail call from system call error and it has been used
> this way in glibc.  It is OK to tail call __syscall_error and ignore the user
> provided return value for syscall error.  I can add a comment to i386
> INLINE_SYSCALL_ERROR_RETURN.

It's the generic header, not the i386 one, that needs a detailed comment 
explaining the semantics of the macro so that it is clear what uses of the 
macro are or are not correct, and, similarly, what definitions are or are 
not correct.  That includes:

* The circumstances in which the macro may return directly from the 
function rather than evaluating to an appropriate value.  (I'm not clear 
why this macro needs to contain a return statement at all, rather than 
just expanding to ((type) __syscall_error (resultvar)).  Much the same 
applies to INLINE_SYSCALL_RETURN: why not do (__glibc_unlikely 
(INTERNAL_SYSCALL_ERROR_P (resultvar, )) ? INLINE_SYSCALL_ERROR_RETURN 
(resultvar, type, ) : (type) resultvar) after the assignment to resultvar?  
They're logically equivalent as long as the macros are only used in return 
statements, do they generate different code?)

* The set of valid values to which the macro may evaluate if it doesn't 
change control flow.

* The set of valid values the macro may return if it returns from the 
calling function.

For example, are the semantics "either return -1 from the calling 
function, or evaluate to VALUE"?  "either return -1 or VALUE from the 
calling function, or evaluate to -1 or VALUE"?  Something else, with 
specific determinate conditions for the return value?

If the semantics make it indeterminate whether -1 or VALUE is returned, 
you need to explain very clearly why this is a useful internal interface, 
as it seems like a very dubious interface to me.

-- 
Joseph S. Myers
joseph@codesourcery.com