Re: [PATCH] Consolidate fallocate/fallocate64 implementations

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 27/06/2016 18:43, Arnd Bergmann wrote:
> On Monday, June 27, 2016 4:34:33 PM CEST Adhemerval Zanella wrote:
>> diff --git a/sysdeps/unix/sysv/linux/fallocate.c b/sysdeps/unix/sysv/linux/fallocate.c
>> index 6a58a5f..e648a73 100644
>> --- a/sysdeps/unix/sysv/linux/fallocate.c
>> +++ b/sysdeps/unix/sysv/linux/fallocate.c
>> @@ -19,17 +19,12 @@
>>  #include <fcntl.h>
>>  #include <sysdep-cancel.h>
>>  
>> +#if __WORDSIZE != 64 || defined (__ASSUME_OFF_DIFF_OFF64)
>>  /* Reserve storage for the data of the file associated with FD.  */
>>  int
>>  fallocate (int fd, int mode, __off_t offset, __off_t len)
>>  {
>>    return SYSCALL_CANCEL (fallocate, fd, mode,
>> +                        SYSCALL_LL (offset), SYSCALL_LL (len));
>>  }
>> +#endif
>> diff --git a/sysdeps/unix/sysv/linux/fallocate64.c b/sysdeps/unix/sysv/linux/fallocate64.c
>> index 8e76d6f..176caa8 100644
>> --- a/sysdeps/unix/sysv/linux/fallocate64.c
>> +++ b/sysdeps/unix/sysv/linux/fallocate64.c
>> @@ -24,14 +24,10 @@
>>  int
>>  fallocate64 (int fd, int mode, __off64_t offset, __off64_t len)
>>  {
>>    return SYSCALL_CANCEL (fallocate, fd, mode,
>> +                        SYSCALL_LL64 (offset), SYSCALL_LL64 (len));
>>  }
>> +
>> +#if __WORDSIZE == 64 && !defined (__ASSUME_OFF_DIFF_OFF64)
>> +weak_alias (fallocate64, fallocate)
>> +#endif
>>
> 
> How should this work for new 32-bit architectures when they do
> not set __ASSUME_OFF_DIFF_OFF64?
> 
> It looks like you have two identical implementations then, though
> you would want to have just one of them plus an alias.
> 
> Maybe the "__WORDSIZE == 64" check can simply be dropped and all
> existing 32-bit architectures set __ASSUME_OFF_DIFF_OFF64 instead?
> 
> 	Arnd
> 

It was something that crossed my mind just after I sent the patch.
And it is not wrong itself, although suboptimal as you noted
(glibc will provide fallocate and fallocate64 which will be same the
implementation).

I think for fallocate we can just use

fallocate.c
#if __WORDSIZE != 64 && !defined __ASSUME_WORDSIZE64_ILP32
/* build fallocate */
#endif

and

fallocate64.c

/* build fallocate64 */
#if __WORDSIZE == 64 || defined __ASSUME_WORDSIZE64_ILP32
weak_alias (fallocate64, fallocate)
#endif

It will avoid building fallocate for ILP32 architectures with
off and off64_t with the same size.