Test p{read,write}64 with offset > 4GB [BZ #20350]

Message ID 20160711210107.GA4251@intel.com
State New, archived
Headers

Commit Message

Lu, Hongjiu July 11, 2016, 9:01 p.m. UTC
  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
  

Comments

Adhemerval Zanella Netto July 12, 2016, 2:17 p.m. UTC | #1
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;
> +}
>
  

Patch

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.
+   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
+
+#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;
+}