Re: [PATCH] work around duff ABIs

[Erik Andersen <andersen@codepoet.org>]

On Sun Oct 20, 2002 at 01:51:09PM +0100, Matthew Wilcox wrote:
> On Sat, Oct 19, 2002 at 10:51:49PM -0600, Erik Andersen wrote:
> > Nonono.  Please see __LONG_LONG_PAIR in /usr/include/endian.h.
> > Your user space code should be doing something like this:
> > 
> >     static inline _syscall5(ssize_t, __syscall_pread, int, fd, void *, buf,
> > 	    size_t, count, off_t, offset_hi, off_t, offset_lo);
> > 
> >     ssize_t __libc_pread(int fd, void *buf, size_t count, off_t offset)
> >     {
> > 	return(__syscall_pread(fd,buf,count,__LONG_LONG_PAIR((off_t)0,offset)));
> >     }
> > 
> >     ssize_t __libc_pread64(int fd, void *buf, size_t count, off64_t offset)
> >     {
> > 	return(__syscall_pread(fd, buf, count,
> > 		    __LONG_LONG_PAIR((off_t)(offset>>32),
> > 		    (off_t)(offset&0xffffffff))));
> >     }
> > 
> > Your patch is going to break GNU libc, uClibc, and anyone else in
> > userspace that is doing pread and pread64 correctly....
> 
> Well... since you just proved that you don't understand the problem,
> I'd hazard a guess that uClibc is also broken, as well as glibc.

I understand the problem very well.  Passing 64 bit stuff via
syscalls is a major pain in the butt.  But your patch is not just
changing hppa and mips -- you are breaking the ABI on x86, arm,
powerpc, etc, etc. etc where it is currently working.  Look very
closely at your patch.  See those endianness ifdefs?  You are
adding endianness specific ifdefs into pread, truncate, and
ftruncate to switch the argument order.  User space is already
doing that too.  At no time on any architecture is the low stuff
passed into arg3.  Ergo, your patch is going to break userspace
where pread and pread64 are now working correctly....

If you want to change the kernel to passing eliminate 64 bit
stuff via syscalls, and instead pass pairs of 32bit entities --
I'm all for that as that would make explicit what user space is
doing anyways.  But don't break binary compatibility for no
reason.  Why make both user-space _and_ kernel space add ifdefs
for endianness?  Make arg3 _always_ contain the hi-bits.

 -Erik

--
Erik B. Andersen             http://codepoet-consulting.com/
--This message was written using 73% post-consumer electrons--