Web lists-archives.com

Re: [PATCH 0/5] Add a new "sparse" tree walk algorithm




On 11/28/2018 5:18 PM, Ævar Arnfjörð Bjarmason wrote:
This is really interesting. I tested this with:

     diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
     index 124b1bafc4..5c7615f06c 100644
     --- a/builtin/pack-objects.c
     +++ b/builtin/pack-objects.c
     @@ -3143 +3143 @@ static void get_object_list(int ac, const char **av)
     -       mark_edges_uninteresting(&revs, show_edge, sparse);
     +       mark_edges_uninteresting(&revs, show_edge, 1);

To emulate having a GIT_TEST_* mode for this, which seems like a good
idea since it turned up a lot of segfaults in pack-objects. I wasn't
able to get a backtrace for that since it always happens indirectly, and
I didn't dig enough to see how to manually invoke it the right way.

Thanks for double-checking this. I had run a similar test in my prototype implementation, but over-simplified some code when rewriting it for submission (and then forgot to re-run that test).

Specifically, these null checks are important:

diff --git a/list-objects.c b/list-objects.c
index 9bb93d1640..7e864b4db8 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -232,6 +232,10 @@ static void add_edge_parents(struct commit *commit,
        for (parents = commit->parents; parents; parents = parents->next) {
                struct commit *parent = parents->item;
                struct tree *tree = get_commit_tree(parent);
+
+               if (!tree)
+                       continue;
+
                oidset_insert(set, &tree->object.oid);

                if (!(parent->object.flags & UNINTERESTING))
@@ -261,6 +265,8 @@ void mark_edges_uninteresting(struct rev_info *revs,

                if (sparse) {
                        struct tree *tree = get_commit_tree(commit);
+                       if (!tree)
+                               continue;

                        if (commit->object.flags & UNINTERESTING)
                                tree->object.flags |= UNINTERESTING;

I will definitely include a GIT_TEST_* variable in v2.

Thanks,

-Stolee