Re: [PATCH 1/6] Add INLINE_SYSCALL_ERROR_RETURN

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Tue, Oct 13, 2015 at 6:01 AM, Florian Weimer <fweimer@redhat.com> wrote:
> 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.

How about INLINE_SYSCALL_PREPARE_ERROR_RETURN?

> 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.

__wur attribute won't help much since compiler won't warn

int foo = INLINE_SYSCALL_PREPARE_ERROR_RETURN;

-- 
H.J.