Handle return code of parse_commit in revision machinery

This fixes a crash in broken repositories where random commits
suddenly disappear.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
Signed-off-by: Junio C Hamano <junkio@cox.net>

diff --git a/revision.c b/revision.c
index e60a26c6bb3f6ec77c3db30bc0b59a5a8e30e604..0125d41136871ad2cd07daaaabf40bd2f902a44f 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -318,7 +318,10 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
        while ((parent = *pp) != NULL) {
                struct commit *p = parent->item;
 
        while ((parent = *pp) != NULL) {
                struct commit *p = parent->item;
 

-               parse_commit(p);
+               if (parse_commit(p) < 0)
+                       die("cannot simplify commit %s (because of %s)",
+                           sha1_to_hex(commit->object.sha1),
+                           sha1_to_hex(p->object.sha1));

                switch (rev_compare_tree(revs, p->tree, commit->tree)) {
                case REV_TREE_SAME:
                        tree_same = 1;
                switch (rev_compare_tree(revs, p->tree, commit->tree)) {
                case REV_TREE_SAME:
                        tree_same = 1;


@@ -347,7 +350,10 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
                                 * IOW, we pretend this parent is a
                                 * "root" commit.
                                 */
                                 * IOW, we pretend this parent is a
                                 * "root" commit.
                                 */

-                               parse_commit(p);
+                               if (parse_commit(p) < 0)
+                                       die("cannot simplify commit %s (invalid %s)",
+                                           sha1_to_hex(commit->object.sha1),
+                                           sha1_to_hex(p->object.sha1));

                                p->parents = NULL;
                        }
                /* fallthrough */
                                p->parents = NULL;
                        }
                /* fallthrough */


@@ -362,14 +368,14 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
                commit->object.flags |= TREECHANGE;
 }
 
                commit->object.flags |= TREECHANGE;
 }
 

-static void add_parents_to_list(struct rev_info *revs, struct commit *commit, struct commit_list **list)
+static int add_parents_to_list(struct rev_info *revs, struct commit *commit, struct commit_list **list)

 {
        struct commit_list *parent = commit->parents;
        unsigned left_flag;
        int add, rest;
 
        if (commit->object.flags & ADDED)
 {
        struct commit_list *parent = commit->parents;
        unsigned left_flag;
        int add, rest;
 
        if (commit->object.flags & ADDED)

-               return;
+               return 0;

        commit->object.flags |= ADDED;
 
        /*
        commit->object.flags |= ADDED;
 
        /*


@@ -388,7 +394,8 @@ static void add_parents_to_list(struct rev_info *revs, struct commit *commit, st
                while (parent) {
                        struct commit *p = parent->item;
                        parent = parent->next;
                while (parent) {
                        struct commit *p = parent->item;
                        parent = parent->next;

-                       parse_commit(p);
+                       if (parse_commit(p) < 0)
+                               return -1;

                        p->object.flags |= UNINTERESTING;
                        if (p->parents)
                                mark_parents_uninteresting(p);
                        p->object.flags |= UNINTERESTING;
                        if (p->parents)
                                mark_parents_uninteresting(p);


@@ -397,7 +404,7 @@ static void add_parents_to_list(struct rev_info *revs, struct commit *commit, st
                        p->object.flags |= SEEN;
                        insert_by_date(p, list);
                }
                        p->object.flags |= SEEN;
                        insert_by_date(p, list);
                }

-               return;
+               return 0;

        }
 
        /*
        }
 
        /*


@@ -409,7 +416,7 @@ static void add_parents_to_list(struct rev_info *revs, struct commit *commit, st
                revs->prune_fn(revs, commit);
 
        if (revs->no_walk)
                revs->prune_fn(revs, commit);
 
        if (revs->no_walk)

-               return;
+               return 0;

 
        left_flag = (commit->object.flags & SYMMETRIC_LEFT);
 
 
        left_flag = (commit->object.flags & SYMMETRIC_LEFT);
 


@@ -418,7 +425,8 @@ static void add_parents_to_list(struct rev_info *revs, struct commit *commit, st
                struct commit *p = parent->item;
 
                parent = parent->next;
                struct commit *p = parent->item;
 
                parent = parent->next;

-               parse_commit(p);
+               if (parse_commit(p) < 0)
+                       return -1;

                p->object.flags |= left_flag;
                if (p->object.flags & SEEN)
                        continue;
                p->object.flags |= left_flag;
                if (p->object.flags & SEEN)
                        continue;


@@ -426,6 +434,7 @@ static void add_parents_to_list(struct rev_info *revs, struct commit *commit, st
                if (add)
                        insert_by_date(p, list);
        }
                if (add)
                        insert_by_date(p, list);
        }

+       return 0;

 }
 
 static void cherry_pick_list(struct commit_list *list)
 }
 
 static void cherry_pick_list(struct commit_list *list)


@@ -508,7 +517,7 @@ static void cherry_pick_list(struct commit_list *list)
        free_patch_ids(&ids);
 }
 
        free_patch_ids(&ids);
 }
 

-static void limit_list(struct rev_info *revs)
+static int limit_list(struct rev_info *revs)

 {
        struct commit_list *list = revs->commits;
        struct commit_list *newlist = NULL;
 {
        struct commit_list *list = revs->commits;
        struct commit_list *newlist = NULL;


@@ -524,7 +533,8 @@ static void limit_list(struct rev_info *revs)
 
                if (revs->max_age != -1 && (commit->date < revs->max_age))
                        obj->flags |= UNINTERESTING;
 
                if (revs->max_age != -1 && (commit->date < revs->max_age))
                        obj->flags |= UNINTERESTING;

-               add_parents_to_list(revs, commit, &list);
+               if (add_parents_to_list(revs, commit, &list) < 0)
+                       return -1;

                if (obj->flags & UNINTERESTING) {
                        mark_parents_uninteresting(commit);
                        if (everybody_uninteresting(list))
                if (obj->flags & UNINTERESTING) {
                        mark_parents_uninteresting(commit);
                        if (everybody_uninteresting(list))


@@ -539,6 +549,7 @@ static void limit_list(struct rev_info *revs)
                cherry_pick_list(newlist);
 
        revs->commits = newlist;
                cherry_pick_list(newlist);
 
        revs->commits = newlist;

+       return 0;

 }
 
 struct all_refs_cb {
 }
 
 struct all_refs_cb {


@@ -1227,7 +1238,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
        return left;
 }
 
        return left;
 }
 

-void prepare_revision_walk(struct rev_info *revs)
+int prepare_revision_walk(struct rev_info *revs)

 {
        int nr = revs->pending.nr;
        struct object_array_entry *e, *list;
 {
        int nr = revs->pending.nr;
        struct object_array_entry *e, *list;


@@ -1249,42 +1260,57 @@ void prepare_revision_walk(struct rev_info *revs)
        free(list);
 
        if (revs->no_walk)
        free(list);
 
        if (revs->no_walk)

-               return;
+               return 0;

        if (revs->limited)
        if (revs->limited)

-               limit_list(revs);
+               if (limit_list(revs) < 0)
+                       return -1;

        if (revs->topo_order)
                sort_in_topological_order_fn(&revs->commits, revs->lifo,
                                             revs->topo_setter,
                                             revs->topo_getter);
        if (revs->topo_order)
                sort_in_topological_order_fn(&revs->commits, revs->lifo,
                                             revs->topo_setter,
                                             revs->topo_getter);

+       return 0;

 }
 
 }
 

-static int rewrite_one(struct rev_info *revs, struct commit **pp)
+enum rewrite_result {
+       rewrite_one_ok,
+       rewrite_one_noparents,
+       rewrite_one_error,
+};
+
+static enum rewrite_result rewrite_one(struct rev_info *revs, struct commit **pp)

 {
        for (;;) {
                struct commit *p = *pp;
                if (!revs->limited)
 {
        for (;;) {
                struct commit *p = *pp;
                if (!revs->limited)

-                       add_parents_to_list(revs, p, &revs->commits);
+                       if (add_parents_to_list(revs, p, &revs->commits) < 0)
+                               return rewrite_one_error;

                if (p->parents && p->parents->next)
                if (p->parents && p->parents->next)

-                       return 0;
+                       return rewrite_one_ok;

                if (p->object.flags & (TREECHANGE | UNINTERESTING))
                if (p->object.flags & (TREECHANGE | UNINTERESTING))

-                       return 0;
+                       return rewrite_one_ok;

                if (!p->parents)
                if (!p->parents)

-                       return -1;
+                       return rewrite_one_noparents;

                *pp = p->parents->item;
        }
 }
 
                *pp = p->parents->item;
        }
 }
 

-static void rewrite_parents(struct rev_info *revs, struct commit *commit)
+static int rewrite_parents(struct rev_info *revs, struct commit *commit)

 {
        struct commit_list **pp = &commit->parents;
        while (*pp) {
                struct commit_list *parent = *pp;
 {
        struct commit_list **pp = &commit->parents;
        while (*pp) {
                struct commit_list *parent = *pp;

-               if (rewrite_one(revs, &parent->item) < 0) {
+               switch (rewrite_one(revs, &parent->item)) {
+               case rewrite_one_ok:
+                       break;
+               case rewrite_one_noparents:

                        *pp = parent->next;
                        continue;
                        *pp = parent->next;
                        continue;

+               case rewrite_one_error:
+                       return -1;

                }
                pp = &parent->next;
        }
                }
                pp = &parent->next;
        }

+       return 0;

 }
 
 static int commit_match(struct commit *commit, struct rev_info *opt)
 }
 
 static int commit_match(struct commit *commit, struct rev_info *opt)


@@ -1320,7 +1346,8 @@ static struct commit *get_revision_1(struct rev_info *revs)
                        if (revs->max_age != -1 &&
                            (commit->date < revs->max_age))
                                continue;
                        if (revs->max_age != -1 &&
                            (commit->date < revs->max_age))
                                continue;

-                       add_parents_to_list(revs, commit, &revs->commits);
+                       if (add_parents_to_list(revs, commit, &revs->commits) < 0)
+                               return NULL;

                }
                if (commit->object.flags & SHOWN)
                        continue;
                }
                if (commit->object.flags & SHOWN)
                        continue;


@@ -1348,8 +1375,8 @@ static struct commit *get_revision_1(struct rev_info *revs)
                                if (!commit->parents || !commit->parents->next)
                                        continue;
                        }
                                if (!commit->parents || !commit->parents->next)
                                        continue;
                        }

-                       if (revs->parents)
-                               rewrite_parents(revs, commit);
+                       if (revs->parents && rewrite_parents(revs, commit) < 0)
+                               return NULL;

                }
                return commit;
        } while (revs->commits);
                }
                return commit;
        } while (revs->commits);

diff --git a/revision.h b/revision.h
index cdf94ad695a1a929ac094a9a42c868e75c1182a5..2845167746ce4632894fce88e840df60513933aa 100644 (file)
--- a/revision.h
+++ b/revision.h
@@ -113,7 +113,7 @@ extern void init_revisions(struct rev_info *revs, const char *prefix);
 extern int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def);
 extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);
 
 extern int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def);
 extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);
 

-extern void prepare_revision_walk(struct rev_info *revs);
+extern int prepare_revision_walk(struct rev_info *revs);

 extern struct commit *get_revision(struct rev_info *revs);
 
 extern void mark_parents_uninteresting(struct commit *commit);
 extern struct commit *get_revision(struct rev_info *revs);
 
 extern void mark_parents_uninteresting(struct commit *commit);