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>

Jim Hill committed May 17, 2015 at 17:41 UTC f6a1e1e288d13472f5f7fe2b907bb8c0bd69a018
2 files changed +27 -1
sha1_file.c
+1 -1
@@ -3119,7 +3119,7 @@ static int index_core(unsigned char *sha1, int fd, size_t size,
3119 int ret;
3120
3121 if (!size) {
3122 - ret = index_mem(sha1, NULL, size, type, path, flags);
3122 + ret = index_mem(sha1, "", size, type, path, flags);
3123 } else if (size <= SMALL_FILE_SIZE) {
3124 char *buf = xmalloc(size);
3125 if (size == read_in_full(fd, buf, size))
t/t0021-conversion.sh
+26
@@ -204,4 +204,30 @@ test_expect_success EXPENSIVE 'filter large file' '
204 ! test -s err
205 '
206
207 +test_expect_success "filter: clean empty file" '
208 + git config filter.in-repo-header.clean "echo cleaned && cat" &&
209 + git config filter.in-repo-header.smudge "sed 1d" &&
210 +
211 + echo "empty-in-worktree filter=in-repo-header" >>.gitattributes &&
212 + >empty-in-worktree &&
213 +
214 + echo cleaned >expected &&
215 + git add empty-in-worktree &&
216 + git show :empty-in-worktree >actual &&
217 + test_cmp expected actual
218 +'
219 +
220 +test_expect_success "filter: smudge empty file" '
221 + git config filter.empty-in-repo.clean "cat >/dev/null" &&
222 + git config filter.empty-in-repo.smudge "echo smudged && cat" &&
223 +
224 + echo "empty-in-repo filter=empty-in-repo" >>.gitattributes &&
225 + echo dead data walking >empty-in-repo &&
226 + git add empty-in-repo &&
227 +
228 + echo smudged >expected &&
229 + git checkout-index --prefix=filtered- empty-in-repo &&
230 + test_cmp expected filtered-empty-in-repo
231 +'
232 +
233 test_done