Re: [PATCH 1/3] linux: Use INTERNAL_SYSCALL on fstatat{64}

[Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org>]

On 15/10/2020 14:09, Adhemerval Zanella wrote:
> 
> 
> On 15/10/2020 12:45, Florian Weimer wrote:
>> * Adhemerval Zanella via Libc-alpha:
>>
>>> +  return INTERNAL_SYSCALL_ERROR_P (r)
>>> +	 ? INLINE_SYSCALL_ERROR_RETURN_VALUE (-r)
>>> +	 : 0;
>>
>> I'm not really happy how the expression -r breaks the INTERNAL_*
>> abstraction.  Do you really have to change the calling convention for
>> __cp_kstat_stat?  I expect that if you don't do that (and make calls to
>> __cp_kstat_stat tail calls), you can preserve the abstraction.
> 
> The INTERNAL_* abstractions is something I would like to get rid of
> since currently INTERNAL_SYSCALL follows the same convention for all
> architectures (meaning that there is no extra auxiliary variable that
> hold whether the syscall has failed, as previously for powerpc and
> sparc) and Linux assumes that errno values are all in the range of
> [-4096UL, -1UL].
> 
> We can now simplify the INTERNAL_SYSCALL tests to check for specific
> errno value without the need to resorting the extra macros.  I really
> see that we can move to something simpler and more readable internal
> APIs as:
> 
>   #define INTERNAL_SYSCALL_CALL (syscall, ...) ...
> 
>   static inline _Bool internal_syscall_failed (unsigned long int val)
>   {
>     return val > -4096UL;
>   }
> 
>   static inline long int __syscall_ret (unsigned long int val)
>   {
>     if (internal_syscall_failed (val))
>       {
>         errno = -val;
>         return -1;
>       }
>     return 0;
>   }
> 
>   #define INLINE_SYSCALL_CALL (...) \
>    __syscall_ret (NTERNAL_SYSCALL_CALL (__VA_ARGS__))
> 
> 
> And so we can write more specific syscall wrappers as:
> 
>   int r = INTERNAL_SYSCALL_CALL (syscall, ...);
>   if (r == 0 || -r == ENOSYS)
>     return __syscall_ret (r);
>   r = INTERNAL_SYSCALL_CALL (syscall_fallback, ...);
>   return __syscall_ret (r);
> 
> Or similar if there is no need to handle errno:
> 
>   return -INTERNAL_SYSCALL_CALL (syscall, ...);
> 

And in long term I would like to move the INTERNAL_SYSCALL_CALL to proper
inline function instead of macros.  It should result a more clean code
and avoid the pitfalls of using some construction with the macros (such
as BZ#25523). I have some patches done, but there are quite extensive...