From patchwork Thu Jul 7 14:03:00 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 13696 Received: (qmail 16544 invoked by alias); 7 Jul 2016 14:03:22 -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 16531 invoked by uid 89); 7 Jul 2016 14:03:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=H*r:sk:15.2016 X-HELO: mail-wm0-f47.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=cxLTbzESiO7j0b1R+Hgf0jDJF+k2k2nwaw1b+pSyMm0=; b=Z+raHUZkqhxcRCPU455XxlSaebtddjP3BVqjgtIHneQ1YcbPOeKCpcr0UQL3ZOiyNQ MQ0Z9jUehlLQcdeEGNpxKq1gVkZlm9Nv3cJpUkcBYFyFk0OE+gclngmf8L8U9cY75hgh 7mGW1eiWhS8TmK1/pBxnNepGsvkkTiTLQM+5AExLeDOEwK2cSr+z9C9/dEV4MB6imYnF 9bn1PIsDNn86FwS0lK5RFQ8qtZzzckN5kwum3R29Eet1y74V2JPU2PZ9iTSGQBjoaOQa uf+af7DMQH6KQX2P9dJibGPKBclPDC8FxyP9wtkJ0Q84YmFhR2YSuv4Bg6sUQF1BYBMZ ZqoA== X-Gm-Message-State: ALyK8tJ0593l0Uv8J1Jj9bYip9xNFCzwgp4HLfKpp6VFKqyVVuN9ZcnqnWQ5XyS/1XNRncD4 X-Received: by 10.194.47.71 with SMTP id b7mr311025wjn.107.1467900188233; Thu, 07 Jul 2016 07:03:08 -0700 (PDT) Subject: Re: [PATCH v2] Fix LO_HI_LONG definition To: Florian Weimer References: <1467834756-21898-1-git-send-email-adhemerval.zanella@linaro.org> <3d5b3ecf-e74f-7c62-093f-962ae40e8627@redhat.com> Cc: libc-alpha@sourceware.org From: Adhemerval Zanella Message-ID: <577E6114.4090004@linaro.org> Date: Thu, 7 Jul 2016 11:03:00 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <3d5b3ecf-e74f-7c62-093f-962ae40e8627@redhat.com> On 07/07/2016 03:56, Florian Weimer wrote: > On 07/06/2016 09:52 PM, Adhemerval Zanella wrote: >> + /* Create a sparse file larger than 4GB to check if offset is handled >> + correctly in p{write,read}v64. */ >> + off_t base_offset = UINT32_MAX; >> + off_t fsize = base_offset + 2048; >> + if (ftruncate (temp_fd, fsize) != 0) >> + { >> + printf ("error: ftruncate (%jd) failed: %m", (intmax_t) fsize); >> + return 1; >> + } > > I don't think this is is necessary. You should be able to write beyond the current EOF and get a sparse file directly. > > I'm not sure if the current tests catch calling convention errors because the read and write offsets could be mangled in the same way. Maybe you can add a test which performs a pwrite with a large offset and check that the file size reported by fstat is as expected? Indeed, I have changed the testcase to check if returned file is indeed what the test expect. diff --git a/misc/tst-preadvwritev-common.c b/misc/tst-preadvwritev-common.c new file mode 100644 index 0000000..9a66a5f --- /dev/null +++ b/misc/tst-preadvwritev-common.c @@ -0,0 +1,112 @@ +/* Common definitions for preadv and pwritev. + 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 + +static void do_prepare (void); +#define PREPARE(argc, argv) do_prepare () +static int do_test (void); +#define TEST_FUNCTION do_test () +#include "test-skeleton.c" + +static char *temp_filename; +static int temp_fd; + +static void +do_prepare (void) +{ + temp_fd = create_temp_file ("tst-preadvwritev.", &temp_filename); + if (temp_fd == -1) + { + printf ("cannot create temporary file: %m\n"); + exit (1); + } +} + +#define FAIL(str) \ + do { printf ("error: %s (line %d)\n", str, __LINE__); return 1; } while (0) + +static int +do_test_with_offset (off_t offset) +{ + struct iovec iov[2]; + ssize_t ret; + + char buf1[32]; + char buf2[64]; + + memset (buf1, 0xf0, sizeof buf1); + memset (buf2, 0x0f, sizeof buf2); + + /* Write two buffer with 32 and 64 bytes respectively. */ + memset (iov, 0, sizeof iov); + iov[0].iov_base = buf1; + iov[0].iov_len = sizeof buf1; + iov[1].iov_base = buf2; + iov[1].iov_len = sizeof buf2; + + ret = pwritev (temp_fd, iov, 2, offset); + if (ret == -1) + FAIL ("first pwritev returned -1"); + if (ret != (sizeof buf1 + sizeof buf2)) + FAIL ("first pwritev returned an unexpected value"); + + ret = pwritev (temp_fd, iov, 2, sizeof buf1 + sizeof buf2 + offset); + if (ret == -1) + FAIL ("second pwritev returned -1"); + if (ret != (sizeof buf1 + sizeof buf2)) + FAIL ("second pwritev returned an unexpected value"); + + char buf3[32]; + char buf4[64]; + + memset (buf3, 0x0f, sizeof buf3); + memset (buf4, 0xf0, sizeof buf4); + + iov[0].iov_base = buf3; + iov[0].iov_len = sizeof buf3; + iov[1].iov_base = buf4; + iov[1].iov_len = sizeof buf4; + + /* Now read two buffer with 32 and 64 bytes respectively. */ + ret = preadv (temp_fd, iov, 2, offset); + if (ret == -1) + FAIL ("first preadv returned -1"); + if (ret != (sizeof buf3 + sizeof buf4)) + FAIL ("first preadv returned an unexpected value"); + + if (memcmp (buf1, buf3, sizeof buf1) != 0) + FAIL ("first buffer from first preadv different than expected"); + if (memcmp (buf2, buf4, sizeof buf2) != 0) + FAIL ("second buffer from first preadv different than expected"); + + ret = preadv (temp_fd, iov, 2, sizeof buf3 + sizeof buf4 + offset); + if (ret == -1) + FAIL ("second preadv returned -1"); + if (ret != (sizeof buf3 + sizeof buf4)) + FAIL ("second preadv returned an unexpected value"); + + /* And compare the buffers read and written to check if there are equal. */ + if (memcmp (buf1, buf3, sizeof buf1) != 0) + FAIL ("first buffer from second preadv different than expected"); + if (memcmp (buf2, buf4, sizeof buf2) != 0) + FAIL ("second buffer from second preadv different than expected"); + + return 0; +} + diff --git a/misc/tst-preadvwritev.c b/misc/tst-preadvwritev.c index 08deecc..50a80a7 100644 --- a/misc/tst-preadvwritev.c +++ b/misc/tst-preadvwritev.c @@ -16,99 +16,10 @@ License along with the GNU C Library; if not, see . */ -#include - -/* Allow testing of the 64-bit versions as well. */ -#ifndef PREADV -# define PREADV preadv -# define PWRITEV pwritev -#endif - -static void do_prepare (void); -static int do_test (void); -#define PREPARE(argc, argv) do_prepare () -#define TEST_FUNCTION do_test () -#include "../test-skeleton.c" - -static char *temp_filename; -static int temp_fd; - -void -do_prepare (void) -{ - temp_fd = create_temp_file ("tst-PREADVwritev.", &temp_filename); - if (temp_fd == -1) - { - printf ("cannot create temporary file: %m\n"); - exit (1); - } -} - -#define FAIL(str) \ - do { printf ("error: %s (line %d)\n", str, __LINE__); return 1; } while (0) +#include "tst-preadvwritev-common.c" int do_test (void) { - struct iovec iov[2]; - ssize_t ret; - - char buf1[32]; - char buf2[64]; - - memset (buf1, 0xf0, sizeof buf1); - memset (buf2, 0x0f, sizeof buf2); - - memset (iov, 0, sizeof iov); - iov[0].iov_base = buf1; - iov[0].iov_len = sizeof buf1; - iov[1].iov_base = buf2; - iov[1].iov_len = sizeof buf2; - - ret = PWRITEV (temp_fd, iov, 2, 0); - if (ret == -1) - FAIL ("first PWRITEV returned -1"); - if (ret != (sizeof buf1 + sizeof buf2)) - FAIL ("first PWRITEV returned an unexpected value"); - - ret = PWRITEV (temp_fd, iov, 2, sizeof buf1 + sizeof buf2); - if (ret == -1) - FAIL ("second PWRITEV returned -1"); - if (ret != (sizeof buf1 + sizeof buf2)) - FAIL ("second PWRITEV returned an unexpected value"); - - char buf3[32]; - char buf4[64]; - - memset (buf3, 0x0f, sizeof buf3); - memset (buf4, 0xf0, sizeof buf4); - - iov[0].iov_base = buf3; - iov[0].iov_len = sizeof buf3; - iov[1].iov_base = buf4; - iov[1].iov_len = sizeof buf4; - - ret = PREADV (temp_fd, iov, 2, 0); - if (ret == -1) - FAIL ("first PREADV returned -1"); - if (ret != (sizeof buf3 + sizeof buf4)) - FAIL ("first PREADV returned an unexpected value"); - - if (memcmp (buf1, buf3, sizeof buf1) != 0) - FAIL ("first buffer from first PREADV different than expected"); - if (memcmp (buf2, buf4, sizeof buf2) != 0) - FAIL ("second buffer from first PREADV different than expected"); - - ret = PREADV (temp_fd, iov, 2, sizeof buf3 + sizeof buf4); - if (ret == -1) - FAIL ("second PREADV returned -1"); - if (ret != (sizeof buf3 + sizeof buf4)) - FAIL ("second PREADV returned an unexpected value"); - - if (memcmp (buf1, buf3, sizeof buf1) != 0) - FAIL ("first buffer from second PREADV different than expected"); - if (memcmp (buf2, buf4, sizeof buf2) != 0) - FAIL ("second buffer from second PREADV different than expected"); - - return 0; + return do_test_with_offset (0); } diff --git a/misc/tst-preadvwritev64.c b/misc/tst-preadvwritev64.c index ff6e134..eafe5e9 100644 --- a/misc/tst-preadvwritev64.c +++ b/misc/tst-preadvwritev64.c @@ -16,7 +16,36 @@ License along with the GNU C Library; if not, see . */ -#define PREADV preadv64 -#define PWRITEV pwritev64 +#define _FILE_OFFSET_BITS 64 +#include "tst-preadvwritev-common.c" -#include "tst-preadvwritev.c" +int +do_test (void) +{ + int ret; + + ret = do_test_with_offset (0); + + /* Create a sparse file larger than 4GB to check if offset is handled + correctly in p{write,read}v64. */ + off_t base_offset = UINT32_MAX + 2048LL; + ret += do_test_with_offset (base_offset); + + struct stat st; + if (fstat (temp_fd, &st) == -1) + { + printf ("error: fstat on temporary file failed: %m"); + return 1; + } + + /* The total size should base_offset plus 2 * 96. */ + off_t expected_value = base_offset + (2 * (96LL)); + if (st.st_size != expected_value) + { + printf ("error: file size different than expected (%jd != %jd)\n", + (intmax_t) expected_value, (intmax_t) st.st_size); + return 1; + } + + return ret; +} diff --git a/sysdeps/unix/sysv/linux/sysdep.h b/sysdeps/unix/sysv/linux/sysdep.h index 8c9e62e..a469f57 100644 --- a/sysdeps/unix/sysv/linux/sysdep.h +++ b/sysdeps/unix/sysv/linux/sysdep.h @@ -49,10 +49,6 @@ #endif /* Provide a macro to pass the off{64}_t argument on p{readv,writev}{64}. */ -#if __WORDSIZE == 64 || defined __ASSUME_WORDSIZE64_ILP32 -# define LO_HI_LONG(val) (val) -#else -# define LO_HI_LONG(val) \ - (long) (val), \ - (long) (((uint64_t) (val)) >> 32) -#endif +#define LO_HI_LONG(val) \ + (long) (val), \ + (long) (((uint64_t) (val)) >> 32)