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

From: "jeff.liu" <[email-masked]>
Date: Sun, 26 Sep 2010 16:53:02 +0800

Hi Jim,

Thanks for your prompt response, I will fix this issue when all review done.

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;
> }