sha1_file: pass empty buffer to index empty file

`git add` of an empty file with a filter pops complaints from
`copy_fd` about a bad file descriptor.

This traces back to these lines in sha1_file.c:index_core:

if (!size) {
ret = index_mem(sha1, NULL, size, type, path, flags);

The problem here is that content to be added to the index can be
supplied from an fd, or from a memory buffer, or from a pathname. This
call is supplying a NULL buffer pointer and a zero size.

Downstream logic takes the complete absence of a buffer to mean the
data is to be found elsewhere -- for instance, these, from convert.c:

if (params->src) {
write_err = (write_in_full(child_process.in, params->src, params->size) < 0);
} else {
write_err = copy_fd(params->fd, child_process.in);
}

~If there's a buffer, write from that, otherwise the data must be coming
from an open fd.~

Perfectly reasonable logic in a routine that's going to write from
either a buffer or an fd.

So change `index_core` to supply an empty buffer when indexing an empty
file.

There's a patch out there that instead changes the logic quoted above to
take a `-1` fd to mean "use the buffer", but it seems to me that the
distinction between a missing buffer and an empty one carries intrinsic
semantics, where the logic change is adapting the code to handle
incorrect arguments.

Signed-off-by: Jim Hill <gjthill@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sha1_file.c b/sha1_file.c
index a38854ce553c1e59294d5542a37a5404ccd66dc5..6e2d6ec65b7e80f8e8800380146f4ba61cead190 100644 (file)
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3119,7 +3119,7 @@ static int index_core(unsigned char *sha1, int fd, size_t size,
        int ret;
 
        if (!size) {
-               ret = index_mem(sha1, NULL, size, type, path, flags);
+               ret = index_mem(sha1, "", size, type, path, flags);
        } else if (size <= SMALL_FILE_SIZE) {
                char *buf = xmalloc(size);
                if (size == read_in_full(fd, buf, size))

diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index b92e6cb0469ebb2e328c490cba9b8aaa7c1dbb84..b778faf1c85ac5cbaf02a982bef90e2dae19b7d8 100755 (executable)
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -204,4 +204,30 @@ test_expect_success EXPENSIVE 'filter large file' '
        ! test -s err
 '
 
+test_expect_success "filter: clean empty file" '
+       git config filter.in-repo-header.clean  "echo cleaned && cat" &&
+       git config filter.in-repo-header.smudge "sed 1d" &&
+
+       echo "empty-in-worktree    filter=in-repo-header" >>.gitattributes &&
+       >empty-in-worktree &&
+
+       echo cleaned >expected &&
+       git add empty-in-worktree &&
+       git show :empty-in-worktree >actual &&
+       test_cmp expected actual
+'
+
+test_expect_success "filter: smudge empty file" '
+       git config filter.empty-in-repo.clean "cat >/dev/null" &&
+       git config filter.empty-in-repo.smudge "echo smudged && cat" &&
+
+       echo "empty-in-repo filter=empty-in-repo" >>.gitattributes &&
+       echo dead data walking >empty-in-repo &&
+       git add empty-in-repo &&
+
+       echo smudged >expected &&
+       git checkout-index --prefix=filtered- empty-in-repo &&
+       test_cmp expected filtered-empty-in-repo
+'
+
 test_done