This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Test p{read,write}64 with offset > 4GB [BZ #20350]
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Tue, 12 Jul 2016 08:34:01 -0700
- Subject: Re: [PATCH] Test p{read,write}64 with offset > 4GB [BZ #20350]
- Authentication-results: sourceware.org; auth=none
- References: <20160711210107.GA4251@intel.com> <5784FC04.60904@linaro.org>
On Tue, Jul 12, 2016 at 7:17 AM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> LGTM with 2 remarks below.
>
> On 11/07/2016 22:01, H.J. Lu wrote:
>> Test p{read,write}64 with offset > 4GB. Since it is not an error for a
>> successful pread/pwrite call to transfer fewer bytes than requested, we
>> should check if the return value is -1. No need to close and unlink
>> temporary file, which is handled by test-skeleton.c.
>>
>> Tested on x86-64 and i686. OK for trunk?
>>
>> H.J.
>> ---
>> [BZ #20350]
>> * posix/tst-preadwrite.c: Renamed to ...
>> * posix/tst-preadwrite-common.c: This.
>> (do_prepare): Make it static and remove function arguments.
>> (do_test): Likewise.
>> (PREPARE): Updated.
>> (TEST_FUNCTION): New.
>> (name): Make it static.
>> (fd): Likewise.
>> (do_prepare): Use create_temp_file.
>> (do_test): Renamed to ...
>> (do_test_with_offset): This. Make it static and accept offset.
>> Properly check return value of PWRITE and PREAD. Return bytes
>> read. Don't close fd nor unlink name.
>> * posix/tst-preadwrite.c: Rewrite.
>> * posix/tst-preadwrite64.c: Likewise.
>> ---
>> posix/tst-preadwrite-common.c | 96 +++++++++++++++++++++++++++++++++++++++++++
>> posix/tst-preadwrite.c | 87 ++-------------------------------------
>> posix/tst-preadwrite64.c | 40 +++++++++++++++++-
>> 3 files changed, 138 insertions(+), 85 deletions(-)
>> create mode 100644 posix/tst-preadwrite-common.c
>>
>> diff --git a/posix/tst-preadwrite-common.c b/posix/tst-preadwrite-common.c
>> new file mode 100644
>> index 0000000..67a67af
>> --- /dev/null
>> +++ b/posix/tst-preadwrite-common.c
>> @@ -0,0 +1,96 @@
>> +/* Common definitions for pread and pwrite.
>> + Copyright (C) 1998-2016 Free Software Foundation, Inc.
>
> I think it should be just 2016.
>
>> + This file is part of the GNU C Library.
>> +
>> + The GNU C Library is free software; you can redistribute it and/or
>> + modify it under the terms of the GNU Lesser General Public
>> + License as published by the Free Software Foundation; either
>> + version 2.1 of the License, or (at your option) any later version.
>> +
>> + The GNU C Library is distributed in the hope that it will be useful,
>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + Lesser General Public License for more details.
>> +
>> + You should have received a copy of the GNU Lesser General Public
>> + License along with the GNU C Library; if not, see
>> + <http://www.gnu.org/licenses/>. */
>> +
>> +#include <errno.h>
>> +#include <error.h>
>> +#include <string.h>
>> +#include <unistd.h>
>> +
>> +
>> +/* Allow testing of the 64-bit versions as well. */
>> +#ifndef PREAD
>> +# define PREAD pread
>> +# define PWRITE pwrite
>> +#endif
>
> It we define _FILE_OFFSET_BITS to 64 in tst-preadwrite64, should we still
> use pread64? Could we just use the plain pread/pwrite instead and avoid
> the name redefinition?
>
This is what I checked in.
Thanks.
--
H.J.
From cea26bd847cc748b29afbc129ab925451bfa2ba6 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 11 Jul 2016 12:53:25 -0700
Subject: [PATCH] Test p{read,write}64 with offset > 4GB
Test p{read,write}64 with offset > 4GB. Since it is not an error for a
successful pread/pwrite call to transfer fewer bytes than requested, we
should check if the return value is -1. No need to close and unlink
temporary file, which is handled by test-skeleton.c.
[BZ #20350]
* posix/tst-preadwrite.c: Renamed to ...
* posix/tst-preadwrite-common.c: This.
(PREAD): Removed.
(PWRITE): Likewise.
(STRINGIFY): Likewise.
(STRINGIFY2): Likewise.
(do_prepare): Make it static and remove function arguments.
(do_test): Likewise.
(PREPARE): Updated.
(TEST_FUNCTION): New.
(name): Make it static.
(fd): Likewise.
(do_prepare): Use create_temp_file.
(do_test): Renamed to ...
(do_test_with_offset): This. Make it static and accept offset.
Properly check return value of PWRITE and PREAD. Return bytes
read. Don't close fd nor unlink name.
* posix/tst-preadwrite.c: Rewrite.
* posix/tst-preadwrite64.c: Likewise.
---
posix/tst-preadwrite-common.c | 85 ++++++++++++++++++++++++++++++++++++++++++
posix/tst-preadwrite.c | 87 ++-----------------------------------------
posix/tst-preadwrite64.c | 40 ++++++++++++++++++--
3 files changed, 125 insertions(+), 87 deletions(-)
create mode 100644 posix/tst-preadwrite-common.c
diff --git a/posix/tst-preadwrite-common.c b/posix/tst-preadwrite-common.c
new file mode 100644
index 0000000..533fb93
--- /dev/null
+++ b/posix/tst-preadwrite-common.c
@@ -0,0 +1,85 @@
+/* Common definitions for pread and pwrite.
+ Copyright (C) 2016 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <errno.h>
+#include <error.h>
+#include <string.h>
+#include <unistd.h>
+
+static void do_prepare (void);
+#define PREPARE(argc, argv) do_prepare ()
+static int do_test (void);
+#define TEST_FUNCTION do_test ()
+
+/* We might need a bit longer timeout. */
+#define TIMEOUT 20 /* sec */
+
+/* This defines the `main' function and some more. */
+#include <test-skeleton.c>
+
+/* These are for the temporary file we generate. */
+static char *name;
+static int fd;
+
+static void
+do_prepare (void)
+{
+ fd = create_temp_file ("tst-preadwrite.", &name);
+ if (fd == -1)
+ error (EXIT_FAILURE, errno, "cannot create temporary file");
+}
+
+
+static ssize_t
+do_test_with_offset (off_t offset)
+{
+ char buf[1000];
+ char res[1000];
+ int i;
+ ssize_t ret;
+
+ memset (buf, '\0', sizeof (buf));
+ memset (res, '\xff', sizeof (res));
+
+ if (write (fd, buf, sizeof (buf)) != sizeof (buf))
+ error (EXIT_FAILURE, errno, "during write");
+
+ for (i = 100; i < 200; ++i)
+ buf[i] = i;
+ ret = pwrite (fd, buf + 100, 100, offset + 100);
+ if (ret == -1)
+ error (EXIT_FAILURE, errno, "during pwrite");
+
+ for (i = 450; i < 600; ++i)
+ buf[i] = i;
+ ret = pwrite (fd, buf + 450, 150, offset + 450);
+ if (ret == -1)
+ error (EXIT_FAILURE, errno, "during pwrite");
+
+ ret = pread (fd, res, sizeof (buf) - 50, offset + 50);
+ if (ret == -1)
+ error (EXIT_FAILURE, errno, "during pread");
+
+ if (memcmp (buf + 50, res, ret) != 0)
+ {
+ printf ("error: read of pread != write of pwrite\n");
+ return -1;
+ }
+
+ return ret;
+}
diff --git a/posix/tst-preadwrite.c b/posix/tst-preadwrite.c
index b7c1658..9c9acca 100644
--- a/posix/tst-preadwrite.c
+++ b/posix/tst-preadwrite.c
@@ -1,7 +1,6 @@
/* Tests for pread and pwrite.
Copyright (C) 1998-2016 Free Software Foundation, Inc.
This file is part of the GNU C Library.
- Contributed by Ulrich Drepper <drepper@cygnus.com>, 1998.
The GNU C Library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public
@@ -17,88 +16,10 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <errno.h>
-#include <error.h>
-#include <string.h>
-#include <unistd.h>
+#include "tst-preadwrite-common.c"
-
-/* Allow testing of the 64-bit versions as well. */
-#ifndef PREAD
-# define PREAD pread
-# define PWRITE pwrite
-#endif
-
-#define STRINGIFY(s) STRINGIFY2 (s)
-#define STRINGIFY2(s) #s
-
-/* Prototype for our test function. */
-extern void do_prepare (int argc, char *argv[]);
-extern int do_test (int argc, char *argv[]);
-
-/* We have a preparation function. */
-#define PREPARE do_prepare
-
-/* We might need a bit longer timeout. */
-#define TIMEOUT 20 /* sec */
-
-/* This defines the `main' function and some more. */
-#include <test-skeleton.c>
-
-/* These are for the temporary file we generate. */
-char *name;
-int fd;
-
-void
-do_prepare (int argc, char *argv[])
-{
- size_t name_len;
-
-#define FNAME FNAME2(TRUNCATE)
-#define FNAME2(s) "/" STRINGIFY(s) "XXXXXX"
-
- name_len = strlen (test_dir);
- name = malloc (name_len + sizeof (FNAME));
- if (name == NULL)
- error (EXIT_FAILURE, errno, "cannot allocate file name");
- mempcpy (mempcpy (name, test_dir, name_len), FNAME, sizeof (FNAME));
- add_temp_file (name);
-
- /* Open our test file. */
- fd = mkstemp (name);
- if (fd == -1)
- error (EXIT_FAILURE, errno, "cannot open test file `%s'", name);
-}
-
-
-int
-do_test (int argc, char *argv[])
+static int
+do_test (void)
{
- char buf[1000];
- char res[1000];
- int i;
-
- memset (buf, '\0', sizeof (buf));
- memset (res, '\xff', sizeof (res));
-
- if (write (fd, buf, sizeof (buf)) != sizeof (buf))
- error (EXIT_FAILURE, errno, "during write");
-
- for (i = 100; i < 200; ++i)
- buf[i] = i;
- if (PWRITE (fd, buf + 100, 100, 100) != 100)
- error (EXIT_FAILURE, errno, "during %s", STRINGIFY (PWRITE));
-
- for (i = 450; i < 600; ++i)
- buf[i] = i;
- if (PWRITE (fd, buf + 450, 150, 450) != 150)
- error (EXIT_FAILURE, errno, "during %s", STRINGIFY (PWRITE));
-
- if (PREAD (fd, res, sizeof (buf) - 50, 50) != sizeof (buf) - 50)
- error (EXIT_FAILURE, errno, "during %s", STRINGIFY (PREAD));
-
- close (fd);
- unlink (name);
-
- return memcmp (buf + 50, res, sizeof (buf) - 50);
+ return do_test_with_offset (0) == -1;
}
diff --git a/posix/tst-preadwrite64.c b/posix/tst-preadwrite64.c
index 27425be..00af21a 100644
--- a/posix/tst-preadwrite64.c
+++ b/posix/tst-preadwrite64.c
@@ -1,7 +1,6 @@
/* Tests for pread64 and pwrite64.
Copyright (C) 2000-2016 Free Software Foundation, Inc.
This file is part of the GNU C Library.
- Contributed by Ulrich Drepper <drepper@cygnus.com>, 1998.
The GNU C Library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public
@@ -17,7 +16,40 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#define PREAD pread64
-#define PWRITE pwrite64
+#define _FILE_OFFSET_BITS 64
+#include "tst-preadwrite-common.c"
-#include "tst-preadwrite.c"
+static int
+do_test (void)
+{
+ ssize_t ret;
+
+ ret = do_test_with_offset (0);
+ if (ret == -1)
+ return 1;
+
+ /* Create a sparse file larger than 4GB to check if offset is handled
+ correctly in p{write,read}64. */
+ off_t base_offset = UINT32_MAX + 2048LL;
+ ret = do_test_with_offset (base_offset);
+ if (ret == -1)
+ return 1;
+
+ struct stat st;
+ if (fstat (fd, &st) == -1)
+ {
+ printf ("error: fstat on temporary file failed: %m");
+ return 1;
+ }
+
+ /* The file size should >= base_offset plus bytes read. */
+ off_t expected_value = base_offset + ret;
+ if (st.st_size < expected_value)
+ {
+ printf ("error: file size less than expected (%jd > %jd)\n",
+ (intmax_t) expected_value, (intmax_t) st.st_size);
+ return 1;
+ }
+
+ return 0;
+}
--
2.7.4