This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/6] Add INLINE_SYSCALL_ERROR_RETURN


On 10/13/2015 02:44 PM, H.J. Lu wrote:
> On Tue, Oct 13, 2015 at 5:38 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 10/13/2015 01:19 AM, H.J. Lu wrote:
>>> +/* Set error number and return -1.  A target may choose to return the
>>> +   internal function, __syscall_error, which sets errno and returns -1.  */
>>> +#define INLINE_SYSCALL_ERROR_RETURN(err) \
>>> +  ({                                         \
>>> +    __set_errno (err);                               \
>>> +    -1;                                              \
>>> +  })
>>
>> I think the name is unfortunate because it the macro does not contain a
>> return statement.
>>
>> Perhaps INLINE_SYSCALL_SET_ERRNO would be a better name?
>>
>> Florian
> 
> i386 defines it as
> 
> /* Set error number and return -1.  Return the internal function,
>    __syscall_error, which sets errno from the negative error number
>    and returns -1, to avoid PIC.  */
> #undef INLINE_SYSCALL_ERROR_RETURN
> #define INLINE_SYSCALL_ERROR_RETURN(resultvar) \
>   __syscall_error (-(resultvar))
> 
> This macro should only be used with "return".
> INLINE_SYSCALL_SET_ERRNO may be misleading.
> How about INLINE_SYSCALL_SET_ERRNO_AND_RETURN?

Same problem, the macro does not actually return or alter control flow
in any way.  INLINE_SYSCALL_SET_ERRNO_IN_TAIL or
INLINE_SYSCALL_SET_ERRNO_AT_TAIL would be more appropriate.

You could turn the generic definition into an inline function an use the
__wur attribute, then misuse would be easier to detect.  The comment on
the declaration should point users who use this function for side effect
only to __set_errno.

Florian


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]