[PATCH 3/4] Consolidate pread/pread64 implementations
Adhemerval Zanella
adhemerval.zanella@linaro.org
Tue Feb 23 18:46:00 GMT 2016
On 23-02-2016 14:57, Joseph Myers wrote:
> On Tue, 23 Feb 2016, Adhemerval Zanella wrote:
>
>> +/* The n32 have sizeof(off_t) != sizeof(off64_t) so we can't strong/weak
>> + alias the pread to pread64. We undefine the __ASSUME_WORDSIZE64_ILP32
>> + so two implementation are built. */
>> +#if _MIPS_SIM == _ABIN32
>> +# undef __ASSUME_WORDSIZE64_ILP32
>> #endif
>
> I don't see how this will work, since you still need 64-bit arguments
> passed in a single register, and __ASSUME_WORDSIZE64_ILP32 controls
> SYSCALL_LL* in your patch.
It will since mips p{read,write}.c implementation first include sysdep-cancel.h
and by that it will get the correct SYSCALL_LL{64} definition for the ABI.
The undefine and later include on generic p{read,write} will just block the
build symbol and create the alias for p{read,write}64.
>
> Also, you're including sysdeps/unix/sysv/linux/generic/sysdep.h for
> architectures that don't use the generic ABI. If that's safe, that
> suggests it should just be merged into sysdeps/unix/sysv/linux/sysdep.h
> (in a separate patch with a careful argument for why the merge is safe)
> rather than keeping the headers separate.
I decided to add it on generic sysdep.h because it is where port will get
the definition of __ALIGNMENT_ARG. However I can split the second path
in two, one to include generic sysdep.h and another to define SYSCALL_LL{64}.
>
> The n32 ABI is that 32-bit arguments must be sign-extended to 64-bit when
> passed in registers (even if the arguments are of an unsigned type). So I
> think it should be safe for pread to be aliased to pread64 for n32 (it
> does have to be that way round, the function being defined with the 64-bit
> argument type) - though aliasing functions with incompatible C types may
> be tricky.
The patch is doing what current implementation for pread{64}.c does for mips.
This alias could be a following cleanup patch (which is not the intention
of this patch).
More information about the Libc-alpha
mailing list