[uclibc-ng-devel] preadv/pwritev: fix offset argument type
diff mbox series

Message ID 20190510142353.1977-1-jcmvbkbc@gmail.com
State New
Headers show
Series
  • [uclibc-ng-devel] preadv/pwritev: fix offset argument type
Related show

Commit Message

Max Filippov May 10, 2019, 2:23 p.m. UTC
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(-)

Comments

Yann Sionneau May 13, 2019, 3:55 p.m. UTC | #1
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;
>
Max Filippov May 13, 2019, 6:36 p.m. UTC | #2
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.
Yann Sionneau May 13, 2019, 6:42 p.m. UTC | #3
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.
>
Yann Sionneau May 15, 2019, 3:12 p.m. UTC | #4
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

Patch
diff mbox series

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;