From patchwork Tue Jul 12 15:34:01 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 13748 Received: (qmail 112077 invoked by alias); 12 Jul 2016 15:34:15 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 112061 invoked by uid 89); 12 Jul 2016 15:34:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=16, 40, 1, 85, 1640, transfer X-HELO: mail-qk0-f196.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=5VIDeem1APevIH3i6LtzEuXFkKJZTi2IEcgnN8QShgU=; b=KL83Oz/Obwjn3Ro3p8gyXfvzRHJ4r3JZrN27YAXdcvTh0neXIpY+49pks3dy2Ewts+ ORKcTsUmIhPh32Ktba6Z1f4B4slKl9DABjO8uSpT5XEpnCM2jNJl/UvFfJ8lZdBdTGgq qUSHEe1iUn8gl7C2PLXIGauIGxUo54Z/YKdqMei0qkhKNWmNGWT5hnTLPOous+V8nDoU sqlj/OFRd31q9tK+XTjLNP2NLhXP0knAs6yVHCXLhz2P2Zv4JuVD2Ix5bl9oXpDID/Sr Mk8d4HpGF6tzvllwYM3ZOoihyEi3ci+/NUuKEbOD0AG6GKRHQ7eE/UoQ3QKnqfDRtXZ5 WjAQ== X-Gm-Message-State: ALyK8tJhoyhzIY9iRv4o4AepjA3Qi/TTzrbfTNrcegbp0O1gV2t4Gw7EqQLzmR5DqfpU1LM222YZh8MP4arClQ== X-Received: by 10.55.214.135 with SMTP id p7mr3748123qkl.195.1468337641686; Tue, 12 Jul 2016 08:34:01 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <5784FC04.60904@linaro.org> References: <20160711210107.GA4251@intel.com> <5784FC04.60904@linaro.org> From: "H.J. Lu" Date: Tue, 12 Jul 2016 08:34:01 -0700 Message-ID: Subject: Re: [PATCH] Test p{read,write}64 with offset > 4GB [BZ #20350] To: Adhemerval Zanella Cc: GNU C Library On Tue, Jul 12, 2016 at 7:17 AM, Adhemerval Zanella 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 >> + . */ >> + >> +#include >> +#include >> +#include >> +#include >> + >> + >> +/* 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. From cea26bd847cc748b29afbc129ab925451bfa2ba6 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" 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 + . */ + +#include +#include +#include +#include + +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 + +/* 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 , 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 . */ -#include -#include -#include -#include +#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 - -/* 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 , 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 . */ -#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