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: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: libc-alpha at sourceware dot org
- Date: Tue, 12 Jul 2016 15:17:40 +0100
- Subject: Re: [PATCH] Test p{read,write}64 with offset > 4GB [BZ #20350]
- Authentication-results: sourceware.org; auth=none
- References: <20160711210107.GA4251@intel.com>
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?
> +
> +#define STRINGIFY(s) STRINGIFY2 (s)
> +#define STRINGIFY2(s) #s
> +
> +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 %s", STRINGIFY (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 %s", STRINGIFY (PWRITE));
> +
> + ret = PREAD (fd, res, sizeof (buf) - 50, offset + 50);
> + if (ret == -1)
> + error (EXIT_FAILURE, errno, "during %s", STRINGIFY (PREAD));
> +
> + if (memcmp (buf + 50, res, ret) != 0)
> + {
> + printf ("error: read of %s != write of %s\n", STRINGIFY (PREAD),
> + STRINGIFY (PWRITE));
> + 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..d9379b5 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,44 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> +#define _FILE_OFFSET_BITS 64
> +
> #define PREAD pread64
> #define PWRITE pwrite64
>
> -#include "tst-preadwrite.c"
> +#include "tst-preadwrite-common.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;
> +}
>