GNU bug report logs - #6131
[PATCH]: fiemap support for efficient sparse file copy

Previous Next

Full log

Message #287 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "jeff.liu" <jeff.liu <at> oracle.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: Sunil Mushran <sunil.mushran <at> oracle.com>, Paul Eggert <eggert <at> CS.UCLA.EDU>,
	bug-coreutils <at> gnu.org, Joel Becker <Joel.Becker <at> oracle.com>,
	Tao Ma <tao.ma <at> oracle.com>,
	Pádraig Brady <P <at> draigBrady.com>,
	Chris Mason <chris.mason <at> oracle.com>
Subject: Re: bug#6131: [PATCH]: fiemap support for efficient sparse file copy
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;
> }

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.