{"thread":{"id":"40912","subject":"[PATCH] verify_pack: do not ignore return value of verification function","startedAt":"2015-11-30T22:47:42Z","lastAt":"2015-12-01T23:28:04Z","messageCount":2,"participants":["David Turner","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"273844","messageId":"1448923662-24725-1-git-send-email-dturner@twopensource.com","threadId":"40912","inReplyTo":null,"subject":"[PATCH] verify_pack: do not ignore return value of verification function","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-11-30T22:47:42Z","receivedAt":"2015-11-30T22:47:42Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"In verify_pack, a caller-supplied verification function is called.\nThe function returns an int.  If that return value is non-zero,\nverify_pack should fail.\n\nThe only caller of verify_pack is in builtin/fsck.c, whose verify_fn\nreturns a meaningful error code (which was then ignored).  Now, fsck\nmight return a different error code (with more detail).  This would\nhappen in the unlikely event that a commit or tree that is a valid git\nobject but not a valid instance of its type gets into a pack.\n\nSigned-off-by: David Turner <dturner@twopensource.com>\n---\n pack-check.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/pack-check.c b/pack-check.c\nindex 63a595c..433bd86 100644\n--- a/pack-check.c\n+++ b/pack-check.c\n@@ -126,7 +126,7 @@ static int verify_packfile(struct packed_git *p,\n \t\t\t\t    sha1_to_hex(entries[i].sha1), p->pack_name);\n \t\telse if (fn) {\n \t\t\tint eaten = 0;\n-\t\t\tfn(entries[i].sha1, type, size, data, &eaten);\n+\t\t\terr |= fn(entries[i].sha1, type, size, data, &eaten);\n \t\t\tif (eaten)\n \t\t\t\tdata = NULL;\n \t\t}\n-- \n2.4.2.736.g391bb05-twtrsrc\n"},{"id":"273886","messageId":"20151201232804.GC13245@sigill.intra.peff.net","threadId":"40912","inReplyTo":"1448923662-24725-1-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH] verify_pack: do not ignore return value of verification function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-01T23:28:04Z","receivedAt":"2015-12-01T23:28:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 30, 2015 at 05:47:42PM -0500, David Turner wrote:\n\n> In verify_pack, a caller-supplied verification function is called.\n> The function returns an int.  If that return value is non-zero,\n> verify_pack should fail.\n> \n> The only caller of verify_pack is in builtin/fsck.c, whose verify_fn\n> returns a meaningful error code (which was then ignored).  Now, fsck\n> might return a different error code (with more detail).  This would\n> happen in the unlikely event that a commit or tree that is a valid git\n> object but not a valid instance of its type gets into a pack.\n> \n> Signed-off-by: David Turner <dturner@twopensource.com>\n> ---\n>  pack-check.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n\nThanks, this looks good to me.\n\n-Peff\n"}]}