[PATCH 01/23] linux: Remove INTERNAL_SYSCALL_ERRNO

[Adhemerval Zanella]

On 10/11/2020 08:11, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> diff --git a/sysdeps/unix/sysv/linux/posix_fadvise.c b/sysdeps/unix/sysv/linux/posix_fadvise.c
>> index bada96b697..1191ab3db5 100644
>> --- a/sysdeps/unix/sysv/linux/posix_fadvise.c
>> +++ b/sysdeps/unix/sysv/linux/posix_fadvise.c
>> @@ -60,8 +60,6 @@ posix_fadvise (int fd, off_t offset, off_t len, int advise)
>>  				   SYSCALL_LL (len), advise);
>>  #  endif
>>  # endif
>> -  if (INTERNAL_SYSCALL_ERROR_P (ret))
>> -    return INTERNAL_SYSCALL_ERRNO (ret);
>> -  return 0;
>> +  return INTERNAL_SYSCALL_ERROR_P (ret) ? -ret : 0;
>>  }
>>  #endif /* __OFF_T_MATCHES_OFF64_T  */
>> diff --git a/sysdeps/unix/sysv/linux/posix_fadvise64.c b/sysdeps/unix/sysv/linux/posix_fadvise64.c
>> index 9787ab4c7c..e3726a6a8a 100644
>> --- a/sysdeps/unix/sysv/linux/posix_fadvise64.c
>> +++ b/sysdeps/unix/sysv/linux/posix_fadvise64.c
>> @@ -48,9 +48,7 @@ __posix_fadvise64_l64 (int fd, off64_t offset, off64_t len, int advise)
>>  				   __ALIGNMENT_ARG SYSCALL_LL64 (offset),
>>  				   SYSCALL_LL64 (len), advise);
>>  #endif
>> -  if (!INTERNAL_SYSCALL_ERROR_P (ret))
>> -    return 0;
>> -  return INTERNAL_SYSCALL_ERRNO (ret);
>> +  return INTERNAL_SYSCALL_ERROR_P (ret) ? -ret : 0;
>>  }
>>  
>>  /* The type of the len argument was changed from size_t to off_t in
> 
> These two could just use -ret, I think.  The kernel returns 0 for
> success, right?
> 

Yes, I make this change on the the subsequent patch where I remove the
INTERNAL_SYSCALL_ERROR_P macro.  

>> diff --git a/sysdeps/unix/sysv/linux/posix_fallocate.c b/sysdeps/unix/sysv/linux/posix_fallocate.c
>> index 7238b00038..87532668cd 100644
>> --- a/sysdeps/unix/sysv/linux/posix_fallocate.c
>> +++ b/sysdeps/unix/sysv/linux/posix_fallocate.c
>> @@ -30,7 +30,7 @@ posix_fallocate (int fd, __off_t offset, __off_t len)
>>  				   SYSCALL_LL (offset), SYSCALL_LL (len));
>>    if (! INTERNAL_SYSCALL_ERROR_P (res))
>>      return 0;
>> -  if (INTERNAL_SYSCALL_ERRNO (res) != EOPNOTSUPP)
>> -    return INTERNAL_SYSCALL_ERRNO (res);
>> +  if (res != -EOPNOTSUPP)
>> +    return -res;
>>    return internal_fallocate (fd, offset, len);
>>  }
>> diff --git a/sysdeps/unix/sysv/linux/posix_fallocate64.c b/sysdeps/unix/sysv/linux/posix_fallocate64.c
>> index 2de63ac277..0340357e57 100644
>> --- a/sysdeps/unix/sysv/linux/posix_fallocate64.c
>> +++ b/sysdeps/unix/sysv/linux/posix_fallocate64.c
>> @@ -32,8 +32,8 @@ __posix_fallocate64_l64 (int fd, __off64_t offset, __off64_t len)
>>  				   SYSCALL_LL64 (offset), SYSCALL_LL64 (len));
>>    if (! INTERNAL_SYSCALL_ERROR_P (res))
>>      return 0;
>> -  if (INTERNAL_SYSCALL_ERRNO (res) != EOPNOTSUPP)
>> -    return INTERNAL_SYSCALL_ERRNO (res);
>> +  if (-res != EOPNOTSUPP)
>> +    return -res;
>>    return internal_fallocate64 (fd, offset, len);
>>  }
>>  libc_hidden_def (__posix_fallocate64_l64)
> 
> -res == EOPNOTSUPP is inconsistent with the rest of the patch.

Do you mean change to 'res == -EOPNOTSUPP) ?

> 
>> diff --git a/sysdeps/unix/sysv/linux/pthread_sigqueue.c b/sysdeps/unix/sysv/linux/pthread_sigqueue.c
>> index 4b32be2d64..fbbd9fee20 100644
>> --- a/sysdeps/unix/sysv/linux/pthread_sigqueue.c
>> +++ b/sysdeps/unix/sysv/linux/pthread_sigqueue.c
>> @@ -63,8 +63,7 @@ pthread_sigqueue (pthread_t threadid, int signo, const union sigval value)
>>    /* We have a special syscall to do the work.  */
>>    int val = INTERNAL_SYSCALL_CALL (rt_tgsigqueueinfo, pid, tid, signo,
>>  				   &info);
>> -  return (INTERNAL_SYSCALL_ERROR_P (val)
>> -	  ? INTERNAL_SYSCALL_ERRNO (val) : 0);
>> +  return INTERNAL_SYSCALL_ERROR_P (val) ? -val : 0;
>>  #else
>>    return ENOSYS;
>>  #endif
> 
> This could use plain -val.
> 
>> diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/posix_fadvise64.c b/sysdeps/unix/sysv/linux/s390/s390-32/posix_fadvise64.c
>> index b556a6caae..6199589307 100644
>> --- a/sysdeps/unix/sysv/linux/s390/s390-32/posix_fadvise64.c
>> +++ b/sysdeps/unix/sysv/linux/s390/s390-32/posix_fadvise64.c
>> @@ -43,9 +43,7 @@ __posix_fadvise64_l64 (int fd, off64_t offset, off64_t len, int advise)
>>    parameters.len = len;
>>    parameters.advise = advise;
>>    int ret = INTERNAL_SYSCALL_CALL (fadvise64_64, &parameters);
>> -  if (!INTERNAL_SYSCALL_ERROR_P (ret))
>> -    return 0;
>> -  return INTERNAL_SYSCALL_ERRNO (ret);
>> +  return INTERNAL_SYSCALL_ERROR_P (ret) ? -ret : 0;
>>  }
>>  
>>  #include <shlib-compat.h>
> 
> See above, plain -ret should be okay.
> 
> 
> The patch relies on a GCC extension because in the error case, -res is a
> large unsigned value which cannot be represented in an int.  But I think
> this is okay.

I think with subsequent patches where it moves towards inline functions with
a more well typed interface should make the internal syscall interface more
well-defined.