| Message ID | 20190510142353.1977-1-jcmvbkbc@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [uclibc-ng-devel] preadv/pwritev: fix offset argument type | expand |
It has been merged, it trigged continuous integration on my side. It seems tst-preadvwritev still passes on the following arches: armv7, aarch64 But this test was failing before your commit and still fails for mips32r6: https://uclibc-ng-ci.sionneau.net:8443/job/uclibc-ng-multiarch/arch=mips32r6/22/console Any idea why? On 5/10/19 4:23 PM, Max Filippov wrote: > preadv/pwritev don't provide separate version for 64-bit wide off_t, > and default to 32-bit wide off_t, which results in a mismatch between > declaration and definition for user programs built with > -D_FILE_OFFSET_BITS=64. > Make offset argument of both functions __off64_t. > This fixes test misc/tst-preadvwritev on xtensa. > > Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> > --- > include/sys/uio.h | 4 ++-- > libc/sysdeps/linux/common/preadv.c | 2 +- > libc/sysdeps/linux/common/pwritev.c | 2 +- > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/include/sys/uio.h b/include/sys/uio.h > index aa766f9b1187..330426fec492 100644 > --- a/include/sys/uio.h > +++ b/include/sys/uio.h > @@ -59,7 +59,7 @@ extern ssize_t writev (int __fd, const struct iovec *__iovec, int __count); > This function is a cancellation point and therefore not marked with > __THROW. */ > extern ssize_t preadv (int __fd, const struct iovec *__iovec, int __count, > - off_t __offset) __wur; > + __off64_t __offset) __wur; > > /* Write data pointed by the buffers described by IOVEC, which is a > vector of COUNT 'struct iovec's, to file descriptor FD at the given > @@ -71,7 +71,7 @@ extern ssize_t preadv (int __fd, const struct iovec *__iovec, int __count, > This function is a cancellation point and therefore not marked with > __THROW. */ > extern ssize_t pwritev (int __fd, const struct iovec *__iovec, int __count, > - off_t __offset) __wur; > + __off64_t __offset) __wur; > #endif /* Use misc. */ > > __END_DECLS > diff --git a/libc/sysdeps/linux/common/preadv.c b/libc/sysdeps/linux/common/preadv.c > index fd9dde4b999c..6a07d5df87e0 100644 > --- a/libc/sysdeps/linux/common/preadv.c > +++ b/libc/sysdeps/linux/common/preadv.c > @@ -21,7 +21,7 @@ > > #ifdef __NR_preadv > ssize_t > -preadv (int fd, const struct iovec *vector, int count, off_t offset) > +preadv (int fd, const struct iovec *vector, int count, __off64_t offset) > { > unsigned long pos_l, pos_h; > > diff --git a/libc/sysdeps/linux/common/pwritev.c b/libc/sysdeps/linux/common/pwritev.c > index bef5bcf69b46..f07c40e6de3c 100644 > --- a/libc/sysdeps/linux/common/pwritev.c > +++ b/libc/sysdeps/linux/common/pwritev.c > @@ -21,7 +21,7 @@ > > #ifdef __NR_pwritev > ssize_t > -pwritev (int fd, const struct iovec *vector, int count, off_t offset) > +pwritev (int fd, const struct iovec *vector, int count, __off64_t offset) > { > unsigned long pos_l, pos_h; >
Hi Yann, On Mon, May 13, 2019 at 8:55 AM Yann Sionneau <ysionneau@kalray.eu> wrote: > It has been merged, it trigged continuous integration on my side. > > It seems tst-preadvwritev still passes on the following arches: armv7, > aarch64 > > But this test was failing before your commit and still fails for > mips32r6: > https://uclibc-ng-ci.sionneau.net:8443/job/uclibc-ng-multiarch/arch=mips32r6/22/console > > Any idea why? Looking at the full log of your test here https://uclibc-ng-ci.sionneau.net:8443/job/uclibc-ng-multiarch/arch=mips32r6/22/consoleText I see that buildroot builds uclibc-ng 1.0.31 with two patches on top of it, but without the patch that fixes the preadv/pwritev issue. I ran the test using fixed uclibc version, it passes for me on both little- and big-endian mips32r6 when run under linux-user qemu. I'll submit a patch for the buildroot.
Hi Max, Good catch! Thanks for having a look, I'll fix the CI to use the SHA1 of last commit instead of the last release. The CI is a work in progress and any improvement is welcome. Le 13/05/2019 à 20:36, Max Filippov a écrit : > Hi Yann, > > On Mon, May 13, 2019 at 8:55 AM Yann Sionneau <ysionneau@kalray.eu> wrote: >> It has been merged, it trigged continuous integration on my side. >> >> It seems tst-preadvwritev still passes on the following arches: armv7, >> aarch64 >> >> But this test was failing before your commit and still fails for >> mips32r6: >> https://uclibc-ng-ci.sionneau.net:8443/job/uclibc-ng-multiarch/arch=mips32r6/22/console >> >> Any idea why? > Looking at the full log of your test here > https://uclibc-ng-ci.sionneau.net:8443/job/uclibc-ng-multiarch/arch=mips32r6/22/consoleText > I see that buildroot builds uclibc-ng 1.0.31 with two patches on top of it, > but without the patch that fixes the preadv/pwritev issue. > > I ran the test using fixed uclibc version, it passes for me on both > little- and big-endian mips32r6 when run under linux-user qemu. > > I'll submit a patch for the buildroot. >
Max, I confirm the test now also passes on mips32: https://uclibc-ng-ci.sionneau.net:8443/job/uclibc-ng-multiarch/arch=mips32r6/29/console Now the CI uses the git sha1 of the commit that triggered the build :) Thanks! On 5/13/19 8:42 PM, Yann Sionneau wrote: > Hi Max, > > Good catch! > > Thanks for having a look, I'll fix the CI to use the SHA1 of last commit > instead of the last release. > > The CI is a work in progress and any improvement is welcome. > > Le 13/05/2019 à 20:36, Max Filippov a écrit : >> Hi Yann, >> >> On Mon, May 13, 2019 at 8:55 AM Yann Sionneau <ysionneau@kalray.eu> wrote: >>> It has been merged, it trigged continuous integration on my side. >>> >>> It seems tst-preadvwritev still passes on the following arches: armv7, >>> aarch64 >>> >>> But this test was failing before your commit and still fails for >>> mips32r6: >>> https://uclibc-ng-ci.sionneau.net:8443/job/uclibc-ng-multiarch/arch=mips32r6/22/console >>> >>> Any idea why? >> Looking at the full log of your test here >> https://uclibc-ng-ci.sionneau.net:8443/job/uclibc-ng-multiarch/arch=mips32r6/22/consoleText >> I see that buildroot builds uclibc-ng 1.0.31 with two patches on top of it, >> but without the patch that fixes the preadv/pwritev issue. >> >> I ran the test using fixed uclibc version, it passes for me on both >> little- and big-endian mips32r6 when run under linux-user qemu. >> >> I'll submit a patch for the buildroot. >> > _______________________________________________ > devel mailing list > devel@uclibc-ng.org > https://mailman.uclibc-ng.org/cgi-bin/mailman/listinfo/devel
diff --git a/include/sys/uio.h b/include/sys/uio.h index aa766f9b1187..330426fec492 100644 --- a/include/sys/uio.h +++ b/include/sys/uio.h @@ -59,7 +59,7 @@ extern ssize_t writev (int __fd, const struct iovec *__iovec, int __count); This function is a cancellation point and therefore not marked with __THROW. */ extern ssize_t preadv (int __fd, const struct iovec *__iovec, int __count, - off_t __offset) __wur; + __off64_t __offset) __wur; /* Write data pointed by the buffers described by IOVEC, which is a vector of COUNT 'struct iovec's, to file descriptor FD at the given @@ -71,7 +71,7 @@ extern ssize_t preadv (int __fd, const struct iovec *__iovec, int __count, This function is a cancellation point and therefore not marked with __THROW. */ extern ssize_t pwritev (int __fd, const struct iovec *__iovec, int __count, - off_t __offset) __wur; + __off64_t __offset) __wur; #endif /* Use misc. */ __END_DECLS diff --git a/libc/sysdeps/linux/common/preadv.c b/libc/sysdeps/linux/common/preadv.c index fd9dde4b999c..6a07d5df87e0 100644 --- a/libc/sysdeps/linux/common/preadv.c +++ b/libc/sysdeps/linux/common/preadv.c @@ -21,7 +21,7 @@ #ifdef __NR_preadv ssize_t -preadv (int fd, const struct iovec *vector, int count, off_t offset) +preadv (int fd, const struct iovec *vector, int count, __off64_t offset) { unsigned long pos_l, pos_h; diff --git a/libc/sysdeps/linux/common/pwritev.c b/libc/sysdeps/linux/common/pwritev.c index bef5bcf69b46..f07c40e6de3c 100644 --- a/libc/sysdeps/linux/common/pwritev.c +++ b/libc/sysdeps/linux/common/pwritev.c @@ -21,7 +21,7 @@ #ifdef __NR_pwritev ssize_t -pwritev (int fd, const struct iovec *vector, int count, off_t offset) +pwritev (int fd, const struct iovec *vector, int count, __off64_t offset) { unsigned long pos_l, pos_h;
preadv/pwritev don't provide separate version for 64-bit wide off_t, and default to 32-bit wide off_t, which results in a mismatch between declaration and definition for user programs built with -D_FILE_OFFSET_BITS=64. Make offset argument of both functions __off64_t. This fixes test misc/tst-preadvwritev on xtensa. Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> --- include/sys/uio.h | 4 ++-- libc/sysdeps/linux/common/preadv.c | 2 +- libc/sysdeps/linux/common/pwritev.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)