[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#6131: [PATCH]: fiemap support for efficient sparse file copy
From: |
jeff.liu |
Subject: |
bug#6131: [PATCH]: fiemap support for efficient sparse file copy |
Date: |
Sun, 26 Sep 2010 16:53:02 +0800 |
User-agent: |
Thunderbird 2.0.0.14 (X11/20080505) |
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;
> }