Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:
> On Thu, 26 Apr 2007, Christian wrote:
>
>> Julian Phillips wrote:
>>
>> Loaded symbols for /lib/tls/libnss_dns.so.2
>> #0 decode_tree_entry (desc=0xbff5110c, buf=0x0, size=454) at tree-walk.c:10
>> 10 while ((c = *str++) != ' ') {
>> (gdb) bt
>> #0 decode_tree_entry (desc=0xbff5110c, buf=0x0, size=454) at tree-walk.c:10
>> #1 0x0804b9a8 in mark_tree_uninteresting (tree=0x80d0240) at revision.c:65
>> #2 0x080534cd in main (argc=2, argv=Cannot access memory at address 0x1ca
>> ) at http-push.c:1998
>
> It looks like some strange corruption in http-push.
>
> You seem to have a NULL "buf" pointer (which certainly explains a
> SIGSEGV!), but mark_tree_uninteresting will have done
>
> if (parse_tree(tree) < 0)
> die("bad tree %s", sha1_to_hex(obj->sha1));
>
> init_tree_desc(&desc, tree->buffer, tree->size);
> while (tree_entry(&desc, &entry)) {
> ..
>
>
> and tree_entry() does
>
> *entry = desc->entry;
> update_tree_entry(desc);
>
> and none of that sets "entry->buf" to NULL unless "size" was also zero.
This has been bugging me for quite some time by now.
The call to parse_tree is coming from mark_edge_parents_uninteresting(),
which in turn is called from mark_edges_uninteresting(), both in
http-push.c
However, I think these two functions are cut&paste from old rev-list.c,
which we refactored into list-objects.c some time ago (list-objects.c
has almost identical looking functions).
I admit that I do not care much about http-push, but I looked at
these two functions in list-objects.c; it is not clear why we do
not get the same error while running rev-list --objects-edge
(aka "thin pack transfer").
mark_edges_uninteresting calls mark_tree_uninteresting() on the
tree associated with uninteresting edge commits. I am
reasonably sure that these commits have always been parsed when
prepare_revision_walk() returns. However, it also calls
mark_edge_parents_uninteresting(), trying to make the parent
commit and its tree uninteresting. I do not see how we are
guaranteeing that the parents are parsed (and their trees are
available to us).
While I think your analysis above is correct, which means that
the breakage Julian is seeing (which does not have NULL tree
object) has nothing to do with what I observed above, I'd sleep
better if we had something like this patch:
http-push.c | 2 ++
list-objects.c | 2 ++
2 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/http-push.c b/http-push.c
index e3f7675..156f42e 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1995,6 +1995,8 @@ static void mark_edge_parents_uninteresting(struct commit *commit)
struct commit *parent = parents->item;
if (!(parent->object.flags & UNINTERESTING))
continue;
+ if (!parent->object.parsed)
+ parse_object(parent->object.sha1);
mark_tree_uninteresting(parent->tree);
}
}
diff --git a/list-objects.c b/list-objects.c
index 310f8d3..c2e74f1 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -109,6 +109,8 @@ static void mark_edge_parents_uninteresting(struct commit *commit,
struct commit *parent = parents->item;
if (!(parent->object.flags & UNINTERESTING))
continue;
+ if (!parent->object.parsed)
+ parse_object(parent->object.sha1);
mark_tree_uninteresting(parent->tree);
if (revs->edge_hint && !(parent->object.flags & SHOWN)) {
parent->object.flags |= SHOWN;
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html