[PATCH] fix multiple issues in index-pack

Subsystems: the rest

DORMANTno replies

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH] fix multiple issues in index-pack

From: Nicolas Pitre <hidden>
Date: 2016-06-15 22:45:30

Since commit 9441b61dc5, two issues affected correct behavior of 
index-pack:

 1) The real_type of a delta object is the 'real_type' of its base, not
    the 'type' which can be a "delta type".  Consequence of this is a
    corrupted pack index file which only needs to be recreated with a 
    good index-pack command ('git verify-pack' will flag those).

 2( The code sequence:

        result->data = patch_delta(get_base_data(base), base->obj->size,
                                   delta_data, delta_size, &result->size);

    has two issues of its own since base->obj->size should instead be
    base->size as we want the size of the actual object data and not
    the size of the delta object it is represented by.  Except that 
    simply replacing base->obj->size with base->size won't make the
    code more correct as the C language doesn't enforce a particular 
    ordering for the evaluation of needed arguments for a function call,
    hence base->size could be pushed on the stack before get_base_data()
    which initializes base->size is called.

Signed-off-by: Nicolas Pitre <redacted>
---

Damn... this one was subtle.  And I'm still wondering how the hell the 
test suite is able to pass with this.  I'll try to figure out why and 
come up with better tests.
diff --git a/index-pack.c b/index-pack.c
index 0a917d7..e179bd9 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -514,15 +514,14 @@ static void *get_base_data(struct base_data *c)
 static void resolve_delta(struct object_entry *delta_obj,
 			  struct base_data *base, struct base_data *result)
 {
-	void *delta_data;
-	unsigned long delta_size;
+	void *base_data, *delta_data;
 
-	delta_obj->real_type = base->obj->type;
+	delta_obj->real_type = base->obj->real_type;
 	delta_data = get_data_from_pack(delta_obj);
-	delta_size = delta_obj->size;
+	base_data = get_base_data(base);
 	result->obj = delta_obj;
-	result->data = patch_delta(get_base_data(base), base->obj->size,
-				   delta_data, delta_size, &result->size);
+	result->data = patch_delta(base_data, base->size,
+				   delta_data, delta_obj->size, &result->size);
 	free(delta_data);
 	if (!result->data)
 		bad_object(delta_obj->idx.offset, "failed to apply delta");

Re: [PATCH] fix multiple issues in index-pack

From: Marco Roeland <hidden>
Date: 2016-06-15 22:45:30

On Monday October 20th 2008 at 16:46 Nicolas Pitre wrote:
Since commit 9441b61dc5, two issues affected correct behavior of 
index-pack:

 1) The real_type of a delta object is the 'real_type' of its base, not
    the 'type' which can be a "delta type".  Consequence of this is a
    corrupted pack index file which only needs to be recreated with a 
    good index-pack command ('git verify-pack' will flag those).

 2( The code sequence:

        result->data = patch_delta(get_base_data(base), base->obj->size,
                                   delta_data, delta_size, &result->size);

    has two issues of its own since base->obj->size should instead be
    base->size as we want the size of the actual object data and not
    the size of the delta object it is represented by.  Except that 
    simply replacing base->obj->size with base->size won't make the
    code more correct as the C language doesn't enforce a particular 
    ordering for the evaluation of needed arguments for a function call,
    hence base->size could be pushed on the stack before get_base_data()
    which initializes base->size is called.

Signed-off-by: Nicolas Pitre <redacted>
This patch works for me. Thanks and good detective work.
-- 
Marco Roeland

Re: [PATCH] fix multiple issues in index-pack

From: Jeff King <hidden>
Date: 2016-06-15 22:45:30

On Mon, Oct 20, 2008 at 04:46:19PM -0400, Nicolas Pitre wrote:
 2( The code sequence:

        result->data = patch_delta(get_base_data(base), base->obj->size,
                                   delta_data, delta_size, &result->size);

    has two issues of its own since base->obj->size should instead be
    base->size as we want the size of the actual object data and not
    the size of the delta object it is represented by.  Except that 
This one fixes my problem.

Tested-by: Jeff King <redacted>

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help