# [PATCH] verify_pack: do not ignore return value of verification function 2 messages from 2015-11-30 to 2015-12-01. Participants: David Turner, Jeff King. Thread: https://gitlist.dev/t/40912 ## David Turner, 2015-11-30 22:47 Subject: [PATCH] verify_pack: do not ignore return value of verification function Message-ID: <1448923662-24725-1-git-send-email-dturner@twopensource.com> URL: https://gitlist.dev/e/1448923662-24725-1-git-send-email-dturner%40twopensource.com ``` In verify_pack, a caller-supplied verification function is called. The function returns an int. If that return value is non-zero, verify_pack should fail. The only caller of verify_pack is in builtin/fsck.c, whose verify_fn returns a meaningful error code (which was then ignored). Now, fsck might return a different error code (with more detail). This would happen in the unlikely event that a commit or tree that is a valid git object but not a valid instance of its type gets into a pack. Signed-off-by: David Turner --- pack-check.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pack-check.c b/pack-check.c index 63a595c..433bd86 100644 --- a/pack-check.c +++ b/pack-check.c @@ -126,7 +126,7 @@ static int verify_packfile(struct packed_git *p, sha1_to_hex(entries[i].sha1), p->pack_name); else if (fn) { int eaten = 0; - fn(entries[i].sha1, type, size, data, &eaten); + err |= fn(entries[i].sha1, type, size, data, &eaten); if (eaten) data = NULL; } -- 2.4.2.736.g391bb05-twtrsrc ``` ## Jeff King, 2015-12-01 23:28 Subject: Re: [PATCH] verify_pack: do not ignore return value of verification function Message-ID: <20151201232804.GC13245@sigill.intra.peff.net> URL: https://gitlist.dev/e/20151201232804.GC13245%40sigill.intra.peff.net In-Reply-To: <1448923662-24725-1-git-send-email-dturner@twopensource.com> ``` On Mon, Nov 30, 2015 at 05:47:42PM -0500, David Turner wrote: > In verify_pack, a caller-supplied verification function is called. > The function returns an int. If that return value is non-zero, > verify_pack should fail. > > The only caller of verify_pack is in builtin/fsck.c, whose verify_fn > returns a meaningful error code (which was then ignored). Now, fsck > might return a different error code (with more detail). This would > happen in the unlikely event that a commit or tree that is a valid git > object but not a valid instance of its type gets into a pack. > > Signed-off-by: David Turner > --- > pack-check.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Thanks, this looks good to me. -Peff ```