Re: [coreutils] Patch FIEMAP support for effieicent sparse file copy v1

[jeff.liu]

Joel Becker wrote:
> On Mon, Mar 22, 2010 at 10:41:20PM +0800, jeff.liu wrote:
>> +#ifdef __linux__
>> +# undef FS_IOC_FIEMAP
>> +# define FS_IOC_FIEMAP _IOWR('f', 11, struct fiemap)
> 
>       This is a question for the coreutils style folks.  It would seem
> to me you'd want to get FS_IOC_FIEMAP from a header rather than
> explicitly defining it here.  If the header isn't shipped as needed yet,
> maybe it wants to be detected (#ifndef FS_IOC_FIEMAP ...).  Again,
> conform to coreutils style first.
yes, it should be code as '#ifndef...', this issue will be fixed in the next 
patch.

> 
>> +          uint64_t tot_read;
>> +          char buf[optimal_buf_size];
>> +          while (tot_read < ext_len)
>> +            {
>> +          memset (buf, 0, sizeof(buf));
>> +          ssize_t n_read = read (src_fd, buf, optimal_buf_size);
> 
>               You shouldn't read optimial_buf_size if there is less than
> optimal_buf_size left in the extent.  You'll just be reading into the
> hole and writing out a bunch of zeros.  You probably want:
> 
>             uint64_t tot_read;
>             char buf[optimal_buf_size];
>             size_t to_read = optimal_buf_size;
>             while (tot_read < ext_len)
>              {
>             memset (buf, 0, sizeof(buf));
>             if ((ext_len - tot_read) < to_read)
>              to_read = ext_len - tot_read;
>             ssize_t n_read = read (src_fd, buf, to_read);
> 
Thanks for pointing this out!!
I just wonder why the test results are incorrect for some cases, this is 
definitely the root cause.

> 
> Joel
> 
-Jeff