Re: [PATCH] test-container: Use copy_file_range_compat for cross-device copy [BZ #23597]

["H.J. Lu" <hjl dot tools at gmail dot com>]

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?


-- 
H.J.