git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jim Hill <gjthill@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: [PATCH v3] sha1_file: pass empty buffer to index empty file
Date: Sat, 16 May 2015 13:06:36 -0700	[thread overview]
Message-ID: <1431806796-28902-1-git-send-email-gjthill@gmail.com> (raw)
In-Reply-To: <xmqqa8x4fjf5.fsf@gitster.dls.corp.google.com>

`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>
---
I promise to pay more attention to test quality in the future, thanks for the
patience.

 sha1_file.c           |  2 +-
 t/t0021-conversion.sh | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/sha1_file.c b/sha1_file.c
index f860d67..61e2735 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3186,7 +3186,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 ca7d2a6..bf87e9b 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -216,4 +216,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 true &&
+	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
-- 
2.4.1.4.g08cda19

  reply	other threads:[~2015-05-16 20:06 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-14 17:23 [PATCH] sha1_file: pass empty buffer to index empty file Jim Hill
2015-05-14 18:43 ` Junio C Hamano
2015-05-14 23:17   ` [PATCH v2] " Jim Hill
2015-05-15 18:01     ` Junio C Hamano
2015-05-15 23:31       ` Jim Hill
2015-05-16 18:48         ` Junio C Hamano
2015-05-16 20:06           ` Jim Hill [this message]
2015-05-16 23:22             ` [PATCH v3] " Junio C Hamano
2015-05-17 17:37             ` Junio C Hamano
2015-05-17 19:10               ` Junio C Hamano
2015-05-18  0:41                 ` [PATCH v4] " Jim Hill
2015-05-19  6:37                 ` [PATCH v3] " Jeff King
2015-05-19 18:11                   ` Junio C Hamano
2015-05-19 18:17                     ` Junio C Hamano
2015-05-19 18:35                       ` Junio C Hamano
2015-05-19 19:48                         ` Junio C Hamano
2015-05-19 22:14                           ` Jeff King
2015-05-20 17:03                             ` Junio C Hamano
2015-05-19 19:40                     ` Eric Sunshine
2015-05-19 22:09                     ` Jeff King
2015-05-20 17:25                       ` Junio C Hamano
2015-05-20 17:38                         ` Jeff King
2015-05-14 23:26   ` [PATCH] " Jim Hill

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1431806796-28902-1-git-send-email-gjthill@gmail.com \
    --to=gjthill@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).