Re: [coreutils] Feature: Introduce fiemap copy to cp(1) and mv(1) v2

[jeff.liu]

Jim Meyering wrote:
> jeff.liu wrote:
>> Hello All,
>>
>> Please ignore the previous patchsets, there is an issue I just fixed.
> 
> Please summarize incremental changes (patch and/or text) so we don't
> have to guess or generate them ourselves and inspect.
> 
sorry for the lack of info.

The first change is, for each extent returned, before writing the bytes to the 
dest file,
it should lseek(2) against the dest fd to the same logical offset as well, then 
write the bytes read
into buf.  otherwise, the dest file layout will become mixed.
please check:

+          if (lseek (dest_fd, (off_t) ext_logical, SEEK_SET) < 0LL)
+            {
+              error (0, errno, _("cannot lseek %s"), quote (dst_name));
+              return_val = false;
+            }


another change is the comments for why and when need to set the file size at 
the end of
filemap_copy(). I tried to make the comments easy to understand.
Pls forgive me for my poor english skills.

+  /* If a file ends up with holes, the sum of the last extent logical offset
+     and the read returned size should shorter than the actual size of the 
file.
+     We should sets the file size to ((struct stat) st_buf.st_size).  */


>> The revised version were shown as following:
>>
>> >From b3fc14ca851c6717959a984639f60a5cf6cf05a5 Mon Sep 17 00:00:00 2001
>> From: Jie Liu <address@hidden>
>> Date: Fri, 9 Apr 2010 21:31:27 +0800
> ...
>> +              last = 1;
>> +            }
>> +
>> +          char buf[optimal_buf_size];
>> +          while (0 < ext_len)
>> +            {
>> +          memset (buf, 0, sizeof (buf));
>> +
>> +              /* Avoid reading into the holes if the left extent
>> +                 length is shorter than the optimal buffer size.  */
>> +              if (ext_len < optimal_buf_size)
>> +                optimal_buf_size = ext_len;
>> +
>> +          ssize_t n_read = read (src_fd, buf, optimal_buf_size);
>> +              if (n_read < 0)
>> +                {
> 
> Notice the strange-looking indentation above.
> That is because you've indented with a mix of TABs and spaces.
> If you were to run "make syntax-check", it would fail due
> to the presence of those TABs in indentation.
> 
> Remove them and not only will the test pass, but your quoted
> patches will be more readable, too.
> 
> Please read the guidelines in HACKING.
> Here's the part that tells you to run "make syntax-check":
> 
>     Run "make syntax-check", or even "make distcheck"
>     ================================================
>     Making either of those targets runs many integrity and
>     project-specific policy-conformance tests.  For example, the former
>     ensures that you add no trailing blanks and no uses of certain deprecated
>     functions.  The latter performs all "syntax-check" tests, and also
>     ensures that the build completes with no warnings when using a certain
>     set of gcc -W... options.  Don't even bother running "make distcheck"
>     unless you have a reasonably up to date installation including recent
>     versions of gcc and the linux kernel, and modern GNU tools.

Sorry to make this stupidly mistake again, I will do double check for furture 
patches submition.


Thanks,
-Jeff