This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 3/4] Consolidate pread/pread64 implementations
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: Joseph Myers <joseph at codesourcery dot com>
- Cc: libc-alpha at sourceware dot org, Adhemerval Zanella <adhemerval dot zanella at linaro dot com>
- Date: Tue, 23 Feb 2016 15:42:07 -0300
- Subject: Re: [PATCH 3/4] Consolidate pread/pread64 implementations
- Authentication-results: sourceware.org; auth=none
- References: <1456240627-18774-1-git-send-email-adhemerval dot zanella at linaro dot org> <1456240627-18774-4-git-send-email-adhemerval dot zanella at linaro dot org> <alpine dot DEB dot 2 dot 10 dot 1602231749560 dot 17168 at digraph dot polyomino dot org dot uk>
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).