{"thread":{"id":"15986","subject":"Re: [PATCH] fix multiple issues in index-pack","startedAt":"2008-10-20T20:46:19Z","lastAt":"2008-10-20T21:31:54Z","messageCount":4,"participants":["Marco Roeland","Jeff King","Junio C Hamano","Nicolas Pitre"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"93556","messageId":"alpine.LFD.2.00.0810201609300.26244@xanadu.home","threadId":"15986","inReplyTo":null,"subject":"[PATCH] fix multiple issues in index-pack","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-10-20T20:46:19Z","receivedAt":"2008-10-20T20:46:19Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"Since commit 9441b61dc5, two issues affected correct behavior of \nindex-pack:\n\n 1) The real_type of a delta object is the 'real_type' of its base, not\n the 'type' which can be a \"delta type\". Consequence of this is a\n corrupted pack index file which only needs to be recreated with a \n good index-pack command ('git verify-pack' will flag those).\n\n 2( The code sequence:\n\n result->data = patch_delta(get_base_data(base), base->obj->size,\n delta_data, delta_size, &result->size);\n\n has two issues of its own since base->obj->size should instead be\n base->size as we want the size of the actual object data and not\n the size of the delta object it is represented by. Except that \n simply replacing base->obj->size with base->size won't make the\n code more correct as the C language doesn't enforce a particular \n ordering for the evaluation of needed arguments for a function call,\n hence base->size could be pushed on the stack before get_base_data()\n which initializes base->size is called.\n\nSigned-off-by: Nicolas Pitre \n---\n\nDamn... this one was subtle. And I'm still wondering how the hell the \ntest suite is able to pass with this. I'll try to figure out why and \ncome up with better tests.\n\ndiff --git a/index-pack.c b/index-pack.c\nindex 0a917d7..e179bd9 100644\n--- a/index-pack.c\n+++ b/index-pack.c\n@@ -514,15 +514,14 @@ static void *get_base_data(struct base_data *c)\n static void resolve_delta(struct object_entry *delta_obj,\n \t\t\t struct base_data *base, struct base_data *result)\n {\n-\tvoid *delta_data;\n-\tunsigned long delta_size;\n+\tvoid *base_data, *delta_data;\n \n-\tdelta_obj->real_type = base->obj->type;\n+\tdelta_obj->real_type = base->obj->real_type;\n \tdelta_data = get_data_from_pack(delta_obj);\n-\tdelta_size = delta_obj->size;\n+\tbase_data = get_base_data(base);\n \tresult->obj = delta_obj;\n-\tresult->data = patch_delta(get_base_data(base), base->obj->size,\n-\t\t\t\t delta_data, delta_size, &result->size);\n+\tresult->data = patch_delta(base_data, base->size,\n+\t\t\t\t delta_data, delta_obj->size, &result->size);\n \tfree(delta_data);\n \tif (!result->data)\n \t\tbad_object(delta_obj->idx.offset, \"failed to apply delta\");\n"},{"id":"93549","messageId":"20081020205649.GA21859@coredump.intra.peff.net","threadId":"15986","inReplyTo":"alpine.LFD.2.00.0810201609300.26244@xanadu.home","subject":"Re: [PATCH] fix multiple issues in index-pack","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-10-20T20:56:49Z","receivedAt":"2008-10-20T20:56:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 20, 2008 at 04:46:19PM -0400, Nicolas Pitre wrote:\n\n> 2( The code sequence:\n> \n> result->data = patch_delta(get_base_data(base), base->obj->size,\n> delta_data, delta_size, &result->size);\n> \n> has two issues of its own since base->obj->size should instead be\n> base->size as we want the size of the actual object data and not\n> the size of the delta object it is represented by. Except that \n\nThis one fixes my problem.\n\nTested-by: Jeff King \n\n-Peff\n"},{"id":"93543","messageId":"20081020205925.GA25524@fiberbit.xs4all.nl","threadId":"15986","inReplyTo":"alpine.LFD.2.00.0810201609300.26244@xanadu.home","subject":"Re: [PATCH] fix multiple issues in index-pack","fromName":"Marco Roeland","fromEmail":"marco.roeland@xs4all.nl","sentAt":"2008-10-20T20:59:25Z","receivedAt":"2008-10-20T20:59:25Z","isPatch":true,"sender":{"key":"marco.roeland@xs4all.nl","avatar":null},"body":"On Monday October 20th 2008 at 16:46 Nicolas Pitre wrote:\n\n> Since commit 9441b61dc5, two issues affected correct behavior of \n> index-pack:\n> \n> 1) The real_type of a delta object is the 'real_type' of its base, not\n> the 'type' which can be a \"delta type\". Consequence of this is a\n> corrupted pack index file which only needs to be recreated with a \n> good index-pack command ('git verify-pack' will flag those).\n> \n> 2( The code sequence:\n> \n> result->data = patch_delta(get_base_data(base), base->obj->size,\n> delta_data, delta_size, &result->size);\n> \n> has two issues of its own since base->obj->size should instead be\n> base->size as we want the size of the actual object data and not\n> the size of the delta object it is represented by. Except that \n> simply replacing base->obj->size with base->size won't make the\n> code more correct as the C language doesn't enforce a particular \n> ordering for the evaluation of needed arguments for a function call,\n> hence base->size could be pushed on the stack before get_base_data()\n> which initializes base->size is called.\n> \n> Signed-off-by: Nicolas Pitre \n\nThis patch works for me. Thanks and good detective work.\n-- \nMarco Roeland\n"},{"id":"93552","messageId":"7vskqr2bsl.fsf@gitster.siamese.dyndns.org","threadId":"15986","inReplyTo":"alpine.LFD.2.00.0810201609300.26244@xanadu.home","subject":"Re: [PATCH] fix multiple issues in index-pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-20T21:31:54Z","receivedAt":"2008-10-20T21:31:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre writes:\n\n> Damn... this one was subtle. And I'm still wondering how the hell the \n> test suite is able to pass with this. I'll try to figure out why and \n> come up with better tests.\n\nThanks; much appreciated.\n"}]}