This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] test-container: Use copy_file_range_compat for cross-device copy [BZ #23597]
On 08/31/2018 10:40 AM, H.J. Lu wrote:
> On Fri, Aug 31, 2018 at 5:02 AM, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>> On 30/08/2018 18:26, H.J. Lu wrote:
>>> On Thu, Aug 30, 2018 at 12:10 PM, DJ Delorie <dj@redhat.com> wrote:
>>>> Ah, thanks for figuring that out. I'll add a fallback unless someone
>>>> else feels like doing it before I do ;-)
>>> Something like this?
>>>
>>>
>>> -- H.J.
>>>
>>>
>>> 0001-test-container-Use-copy_file_range_compat-for-cross-.patch
>>>
>>>
>>> From c6b06cc42ed58d6c8f9cdd7e21e89214ce7b2a18 Mon Sep 17 00:00:00 2001
>>> From: "H.J. Lu" <hjl.tools@gmail.com>
>>> Date: Thu, 30 Aug 2018 14:21:14 -0700
>>> Subject: [PATCH] test-container: Use copy_file_range_compat for cross-device
>>> copy [BZ #23597]
>>>
>>> copy_file_range can't be used to copy a file from glibc source directory
>>> to glibc build directory since they may be on different filesystems.
>>> This uses copy_file_range_compat for cross-device copy.
>>>
>>> [BZ #23597]
>>> * io/copy_file_range-compat.c (COPY_FILE_RANGE): Allow
>>> cross-device copies for test-container.c.
>>> * support/test-container.c (COPY_FILE_RANGE_DECL): New.
>>> (COPY_FILE_RANGE): Likewise.
>>> (__libc_lseek64): Likewise.
>>> Include <io/copy_file_range-compat.c>.
>>> (copy_one_file): Call copy_file_range_compat for cross-device
>>> copy.
>>> ---
>>> io/copy_file_range-compat.c | 3 +++
>>> support/test-container.c | 9 ++++++++-
>>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/io/copy_file_range-compat.c b/io/copy_file_range-compat.c
>>> index 4ab22cad19..15860a7480 100644
>>> --- a/io/copy_file_range-compat.c
>>> +++ b/io/copy_file_range-compat.c
>>> @@ -59,12 +59,15 @@ COPY_FILE_RANGE (int infd, __off64_t *pinoff,
>>> __set_errno (EINVAL);
>>> return -1;
>>> }
>>> +#ifndef SUPPORT_TEST_DRIVER_H
>>> + /* Allow cross-device copies for test-container.c. */
>>> if (instat.st_dev != outstat.st_dev)
>>> {
>>> /* Cross-device copies are not supported. */
>>> __set_errno (EXDEV);
>>> return -1;
>>> }
>>> +#endif
>>
>> I not very found of tying libsupport implementation with libc code,
>> even more by supplying an internal symbol with subtle different
>> semantics than the libc one.
>>
>> We don't actually need all the internal error checks or support of
>> copy_file_range, wouldn't be easier to just pread/pwrite the file
>> contents with a much more simpler implementation?
>
> That is what io/copy_file_range-compat.c does. Are you suggesting
> cut and paste, instead of #include?
Not quite.
We are suggesting a unique and simplified copy of those sources to
be placed into the support/ directory as a *.c file that can be
used for support purposes.
Coupling support behaviour and runtime behaviour is a bad choice,
so we should not do it. Even if it means some code duplication.
--
Cheers,
Carlos.