From 5743350f696745a48dfe7976c98dc8eb5c842d72 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Thu, 26 May 2011 15:54:18 +0200 Subject: [PATCH 1/3] rerere.c: diagnose a corrupt MERGE_RR when hitting EOF between TAB and '\0' If we reach EOF after the SHA1-then-TAB, yet before the NUL that terminates each file name, we would fill the file name buffer with \255 bytes resulting from the repeatedly-failing fgetc (returns EOF/-1) and ultimately complain about "filename too long", because no NUL was encountered. Signed-off-by: Jim Meyering Signed-off-by: Junio C Hamano --- rerere.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/rerere.c b/rerere.c index d260843475..283a0024b0 100644 --- a/rerere.c +++ b/rerere.c @@ -42,8 +42,14 @@ static void read_rr(struct string_list *rr) name = xstrdup(buf); if (fgetc(in) != '\t') die("corrupt MERGE_RR"); - for (i = 0; i < sizeof(buf) && (buf[i] = fgetc(in)); i++) - ; /* do nothing */ + for (i = 0; i < sizeof(buf); i++) { + int c = fgetc(in); + if (c < 0) + die("corrupt MERGE_RR"); + buf[i] = c; + if (c == 0) + break; + } if (i == sizeof(buf)) die("filename too long"); string_list_insert(rr, buf)->util = name; From 5dd564895e84eacfc728183fb5a9215665ff59a3 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Thu, 26 May 2011 15:58:16 +0200 Subject: [PATCH 2/3] remove tests of always-false condition * fsck.c (fsck_error_function): Don't test obj->sha1 == 0. It can never be true, since that sha1 member is an array. * transport.c (set_upstreams): Likewise for ref->new_sha1. Signed-off-by: Jim Meyering Signed-off-by: Junio C Hamano --- fsck.c | 2 +- transport.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fsck.c b/fsck.c index 3d05d4a794..c17a538def 100644 --- a/fsck.c +++ b/fsck.c @@ -350,7 +350,7 @@ int fsck_error_function(struct object *obj, int type, const char *fmt, ...) int len; struct strbuf sb = STRBUF_INIT; - strbuf_addf(&sb, "object %s:", obj->sha1?sha1_to_hex(obj->sha1):"(null)"); + strbuf_addf(&sb, "object %s:", sha1_to_hex(obj->sha1)); va_start(ap, fmt); len = vsnprintf(sb.buf + sb.len, strbuf_avail(&sb), fmt, ap); diff --git a/transport.c b/transport.c index 0078660611..26d4e5234a 100644 --- a/transport.c +++ b/transport.c @@ -156,7 +156,7 @@ static void set_upstreams(struct transport *transport, struct ref *refs, continue; if (!ref->peer_ref) continue; - if (!ref->new_sha1 || is_null_sha1(ref->new_sha1)) + if (is_null_sha1(ref->new_sha1)) continue; /* Follow symbolic refs (mainly for HEAD). */ From b1905aeac5aded421cd90f8d264e27bb39672b36 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 26 May 2011 12:28:44 -0400 Subject: [PATCH 3/3] read_gitfile_gently: use ssize_t to hold read result Otherwise, a negative error return becomes a very large read value. We catch this in practice because we compare the expected and actual numbers of bytes (and you are not likely to be reading (size_t)-1 bytes), but this makes the correctness a little more obvious. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- setup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.c b/setup.c index dadc66659a..a975c8b103 100644 --- a/setup.c +++ b/setup.c @@ -272,7 +272,7 @@ const char *read_gitfile_gently(const char *path) const char *slash; struct stat st; int fd; - size_t len; + ssize_t len; if (stat(path, &st)) return NULL;