bug#6131: [PATCH]: fiemap support for efficient sparse file copy

[jeff.liu]

jeff.liu wrote:
> Hi Jim,
> 
> Thanks for your prompt response, I will fix this issue when all review done.
Hi Jim,

For my current implementation, I just have another thought to remove the "char 
*fname" from struct
extent_scan, and add a new item "int errno" to save the errno set by ioctl(2) 
or lseek(2) if either
call failed.
at first, I am intended to use "char *fname" for the debugging purpose inside, 
however, maybe its
better to do such things outside of the module according to the return value 
and errno. this change
can not only reduce the memory allocation for 'fname' but also make a neatly 
open_extent_scan()
interface.

/* Structure used to reserve extent scan information per file.  */
struct extent_scan
{
 ....
  int errno;
  ....
};

void
open_extent_scan (int src_fd, struct extent_scan **scan);


Is it better?

Thanks,
-Jeff

> 
> 
> Regards,
> -Jeff
> 
> Jim Meyering wrote:
>> Hi Jeff,
>>
>> This function has problems:
>>   - the inner "zeros" declaration shadows the outer one
>>       and ends up being useless.
>>   - the "sizeof zeros" resolves to 4 or 8.  obviously not what you intended.
>>
>> ...
>>>  static bool
>>> +write_zeros (int fd, uint64_t n_bytes)
>>>  {
>>> -  bool last = false;
>>> -  union { struct fiemap f; char c[4096]; } fiemap_buf;
>>> -  struct fiemap *fiemap = &fiemap_buf.f;
>>> -  struct fiemap_extent *fm_ext = &fiemap->fm_extents[0];
>>> -  enum { count = (sizeof fiemap_buf - sizeof *fiemap) / sizeof *fm_ext };
>>> -  verify (count != 0);
>>> +  char *zeros = calloc (IO_BUFSIZE, sizeof (char));
>>> +  if (! zeros)
>>> +    {
>>> +      /* Try a small buffer.  */
>>> +      static char zeros[1024];
>>> +    }
>>>
>>> +  while (n_bytes)
>>> +    {
>>> +      uint64_t n = MIN (sizeof zeros, n_bytes);
>>> +      if ((full_write (fd, zeros, n)) != n)
>>> +        return false;
>>> +      n_bytes -= n;
>>> +    }
>>> +
>>> +  return true;
>>> +}
>> Please use the following instead.
>> I'll review the rest tomorrow or Tuesday.
>>
>> static bool
>> write_zeros (int fd, uint64_t n_bytes)
>> {
>>   static char *zeros;
>>   static size_t nz = IO_BUFSIZE;
>>
>>   if (zeros == NULL)
>>     {
>>       static char fallback[1024];
>>       zeros = calloc (nz, 1);
>>       if (zeros == NULL)
>>         {
>>           zeros = fallback;
>>           nz = sizeof fallback;
>>         }
>>     }
>>
>>   while (n_bytes)
>>     {
>>       uint64_t n = MIN (nz, n_bytes);
>>       if ((full_write (fd, zeros, n)) != n)
>>         return false;
>>       n_bytes -= n;
>>     }
>>
>>   return true;
>> }
> 
> 
> 
>