From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
Wolfgang Denk <wd@denx.de>,
git@vger.kernel.org
Subject: [PATCH] fsck: it is OK for a tag and a commit to lack the body
Date: Sun, 28 Jun 2015 11:18:31 -0700 [thread overview]
Message-ID: <xmqqlhf3elxk.fsf_-_@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20150626155248.GB30273@peff.net> (Jeff King's message of "Fri, 26 Jun 2015 11:52:48 -0400")
When fsck validates a commit or a tag object, it scans each line in
the header the object using helper functions such as "start_with()",
etc. that work on a NUL terminated buffer, but before a1e920a0
(index-pack: terminate object buffers with NUL, 2014-12-08), the
validation functions were fed the object data as counted strings,
not necessarily terminated with a NUL. We added a helper function
require_end_of_header() to be called at the beginning of these
validation functions to insist that the object data contains an
empty line before its end. The theory is that the validating
functions will notice and stop when it hits an empty line as a
normal end of header (or a required header line that is missing)
before scanning past the end of potentially not NUL-terminated
buffer.
But the theory forgot that in the older days, Git itself happily
created objects with only the header lines without a body. This
caused Git 2.2 and later to issue an unnecessary warning on some
existing repositories.
With a1e920a0, we do not need to require an empty line (or the body)
in these objects to safely parse and validate them. Drop the
offending "must have an empty line" check from this helper function,
while keeping the other check to make sure that there is no NUL in
the header part of the object, and adjust the name of the helper to
what it does accordingly.
Noticed-by: Wolfgang Denk <wd@denx.de>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* With a proper commit message this time. I did this directly on
top of a1e920a0 (index-pack: terminate object buffers with NUL,
2014-12-08); it has trivial merge conflicts with more recent
codebase, whose resolutions I'll push out later on 'pu'.
fsck.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/fsck.c b/fsck.c
index 88c92e8..3f264e7 100644
--- a/fsck.c
+++ b/fsck.c
@@ -238,8 +238,8 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)
return retval;
}
-static int require_end_of_header(const void *data, unsigned long size,
- struct object *obj, fsck_error error_func)
+static int verify_headers(const void *data, unsigned long size,
+ struct object *obj, fsck_error error_func)
{
const char *buffer = (const char *)data;
unsigned long i;
@@ -255,6 +255,15 @@ static int require_end_of_header(const void *data, unsigned long size,
}
}
+ /*
+ * We did not find double-LF that separates the header
+ * and the body. Not having a body is not a crime but
+ * we do want to see the terminating LF for the last header
+ * line.
+ */
+ if (size && buffer[size - 1] == '\n')
+ return 0;
+
return error_func(obj, FSCK_ERROR, "unterminated header");
}
@@ -305,7 +314,7 @@ static int fsck_commit_buffer(struct commit *commit, const char *buffer,
unsigned parent_count, parent_line_count = 0;
int err;
- if (require_end_of_header(buffer, size, &commit->object, error_func))
+ if (verify_headers(buffer, size, &commit->object, error_func))
return -1;
if (!skip_prefix(buffer, "tree ", &buffer))
@@ -384,7 +393,7 @@ static int fsck_tag_buffer(struct tag *tag, const char *data,
}
}
- if (require_end_of_header(buffer, size, &tag->object, error_func))
+ if (verify_headers(buffer, size, &tag->object, error_func))
goto done;
if (!skip_prefix(buffer, "object ", &buffer)) {
--
2.5.0-rc0-151-g019519d
next prev parent reply other threads:[~2015-06-28 18:18 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-25 15:51 git error in tag ...: unterminated header Wolfgang Denk
2015-06-25 17:32 ` Junio C Hamano
2015-06-25 20:13 ` Wolfgang Denk
2015-06-25 20:24 ` Junio C Hamano
2015-06-25 21:07 ` Johannes Schindelin
2015-06-25 21:21 ` Junio C Hamano
2015-06-25 22:29 ` Junio C Hamano
2015-06-26 8:06 ` Johannes Schindelin
2015-06-26 15:52 ` Jeff King
2015-06-26 17:37 ` Junio C Hamano
2015-06-27 8:57 ` Johannes Schindelin
2015-06-27 18:36 ` Junio C Hamano
2015-06-28 18:18 ` Junio C Hamano [this message]
2015-06-28 18:21 ` [PATCH] fsck: it is OK for a tag and a commit to lack the body Eric Sunshine
2015-06-29 5:12 ` Johannes Schindelin
2015-06-29 5:42 ` Junio C Hamano
2015-06-29 14:51 ` Johannes Schindelin
2015-06-25 20:48 ` git error in tag ...: unterminated header Junio C Hamano
2015-06-25 17:38 ` Junio C Hamano
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=xmqqlhf3elxk.fsf_-_@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=johannes.schindelin@gmx.de \
--cc=peff@peff.net \
--cc=wd@denx.de \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.