Merge branch 'sn/null-pointer-arith-in-mark-tree-uninteresting'
authorJunio C Hamano <gitster@pobox.com>
Fri, 11 Dec 2015 18:41:01 +0000 (10:41 -0800)
committerJunio C Hamano <gitster@pobox.com>
Fri, 11 Dec 2015 18:41:01 +0000 (10:41 -0800)
mark_tree_uninteresting() has code to handle the case where it gets
passed a NULL pointer in its 'tree' parameter, but the function had
'object = &tree->object' assignment before checking if tree is
NULL. This gives a compiler an excuse to declare that tree will
never be NULL and apply a wrong optimization. Avoid it.

* sn/null-pointer-arith-in-mark-tree-uninteresting:
revision.c: fix possible null pointer arithmetic

1  2 
revision.c
diff --combined revision.c
index 2a9463bd670631dcf96ed456ed23e65b7b84f6d4,7f4acad456a3f5adbf2a5828718ab4488d947388..9404a05eeb4adf7e8c1efeed7e16af2c2504aefa
  #include "commit-slab.h"
  #include "dir.h"
  #include "cache-tree.h"
 +#include "bisect.h"
  
  volatile show_early_output_fn_t show_early_output;
  
 +static const char *term_bad;
 +static const char *term_good;
 +
  char *path_name(const struct name_path *path, const char *name)
  {
        const struct name_path *p;
@@@ -38,7 -34,7 +38,7 @@@
        }
        n = xmalloc(len);
        m = n + len - (nlen + 1);
 -      strcpy(m, name);
 +      memcpy(m, name, nlen + 1);
        for (p = path; p; p = p->up) {
                if (p->elem_len) {
                        m -= p->elem_len + 1;
@@@ -86,7 -82,7 +86,7 @@@ void show_object_with_name(FILE *out, s
        leaf.elem = component;
        leaf.elem_len = strlen(component);
  
 -      fprintf(out, "%s ", sha1_to_hex(obj->sha1));
 +      fprintf(out, "%s ", oid_to_hex(&obj->oid));
        show_path_truncated(out, &leaf);
        fputc('\n', out);
  }
@@@ -106,10 -102,10 +106,10 @@@ static void mark_tree_contents_unintere
        struct name_entry entry;
        struct object *obj = &tree->object;
  
 -      if (!has_sha1_file(obj->sha1))
 +      if (!has_object_file(&obj->oid))
                return;
        if (parse_tree(tree) < 0)
 -              die("bad tree %s", sha1_to_hex(obj->sha1));
 +              die("bad tree %s", oid_to_hex(&obj->oid));
  
        init_tree_desc(&desc, tree->buffer, tree->size);
        while (tree_entry(&desc, &entry)) {
  
  void mark_tree_uninteresting(struct tree *tree)
  {
-       struct object *obj = &tree->object;
+       struct object *obj;
  
        if (!tree)
                return;
+       obj = &tree->object;
        if (obj->flags & UNINTERESTING)
                return;
        obj->flags |= UNINTERESTING;
@@@ -153,7 -151,10 +155,7 @@@ void mark_parents_uninteresting(struct 
                commit_list_insert(l->item, &parents);
  
        while (parents) {
 -              struct commit *commit = parents->item;
 -              l = parents;
 -              parents = parents->next;
 -              free(l);
 +              struct commit *commit = pop_commit(&parents);
  
                while (commit) {
                        /*
                         * it is popped next time around, we won't be trying
                         * to parse it and get an error.
                         */
 -                      if (!has_sha1_file(commit->object.sha1))
 +                      if (!has_object_file(&commit->object.oid))
                                commit->object.parsed = 1;
  
                        if (commit->object.flags & UNINTERESTING)
@@@ -282,11 -283,11 +284,11 @@@ static struct commit *handle_commit(str
                        add_pending_object(revs, object, tag->tag);
                if (!tag->tagged)
                        die("bad tag");
 -              object = parse_object(tag->tagged->sha1);
 +              object = parse_object(tag->tagged->oid.hash);
                if (!object) {
                        if (flags & UNINTERESTING)
                                return NULL;
 -                      die("bad object %s", sha1_to_hex(tag->tagged->sha1));
 +                      die("bad object %s", oid_to_hex(&tag->tagged->oid));
                }
                object->flags |= flags;
                /*
@@@ -510,7 -511,7 +512,7 @@@ static int rev_compare_tree(struct rev_
  
        tree_difference = REV_TREE_SAME;
        DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
 -      if (diff_tree_sha1(t1->object.sha1, t2->object.sha1, "",
 +      if (diff_tree_sha1(t1->object.oid.hash, t2->object.oid.hash, "",
                           &revs->pruning) < 0)
                return REV_TREE_DIFFERENT;
        return tree_difference;
@@@ -526,7 -527,7 +528,7 @@@ static int rev_same_tree_as_empty(struc
  
        tree_difference = REV_TREE_SAME;
        DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
 -      retval = diff_tree_sha1(NULL, t1->object.sha1, "", &revs->pruning);
 +      retval = diff_tree_sha1(NULL, t1->object.oid.hash, "", &revs->pruning);
  
        return retval >= 0 && (tree_difference == REV_TREE_SAME);
  }
@@@ -610,7 -611,7 +612,7 @@@ static unsigned update_treesame(struct 
  
                st = lookup_decoration(&revs->treesame, &commit->object);
                if (!st)
 -                      die("update_treesame %s", sha1_to_hex(commit->object.sha1));
 +                      die("update_treesame %s", oid_to_hex(&commit->object.oid));
                relevant_parents = 0;
                relevant_change = irrelevant_change = 0;
                for (p = commit->parents, n = 0; p; n++, p = p->next) {
@@@ -708,8 -709,8 +710,8 @@@ static void try_to_simplify_commit(stru
                }
                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));
 +                          oid_to_hex(&commit->object.oid),
 +                          oid_to_hex(&p->object.oid));
                switch (rev_compare_tree(revs, p, commit)) {
                case REV_TREE_SAME:
                        if (!revs->simplify_history || !relevant_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));
 +                                          oid_to_hex(&commit->object.oid),
 +                                          oid_to_hex(&p->object.oid));
                                p->parents = NULL;
                        }
                /* fallthrough */
                                irrelevant_change = 1;
                        continue;
                }
 -              die("bad tree compare for commit %s", sha1_to_hex(commit->object.sha1));
 +              die("bad tree compare for commit %s", oid_to_hex(&commit->object.oid));
        }
  
        /*
@@@ -1099,10 -1100,14 +1101,10 @@@ static int limit_list(struct rev_info *
        }
  
        while (list) {
 -              struct commit_list *entry = list;
 -              struct commit *commit = list->item;
 +              struct commit *commit = pop_commit(&list);
                struct object *obj = &commit->object;
                show_early_output_fn_t show;
  
 -              list = list->next;
 -              free(entry);
 -
                if (commit == interesting_cache)
                        interesting_cache = NULL;
  
@@@ -1189,7 -1194,7 +1191,7 @@@ static void add_rev_cmdline_list(struc
  {
        while (commit_list) {
                struct object *object = &commit_list->item->object;
 -              add_rev_cmdline(revs, object, sha1_to_hex(object->sha1),
 +              add_rev_cmdline(revs, object, oid_to_hex(&object->oid),
                                whence, flags);
                commit_list = commit_list->next;
        }
@@@ -1378,7 -1383,7 +1380,7 @@@ static int add_parents_only(struct rev_
                        break;
                if (!((struct tag*)it)->tagged)
                        return 0;
 -              hashcpy(sha1, ((struct tag*)it)->tagged->sha1);
 +              hashcpy(sha1, ((struct tag*)it)->tagged->oid.hash);
        }
        if (it->type != OBJ_COMMIT)
                return 0;
@@@ -1435,7 -1440,7 +1437,7 @@@ static void add_pending_commit_list(str
        while (commit_list) {
                struct object *object = &commit_list->item->object;
                object->flags |= flags;
 -              add_pending_object(revs, object, sha1_to_hex(object->sha1));
 +              add_pending_object(revs, object, oid_to_hex(&object->oid));
                commit_list = commit_list->next;
        }
  }
@@@ -1555,10 -1560,10 +1557,10 @@@ int handle_revision_arg(const char *arg
  
                                a = (a_obj->type == OBJ_COMMIT
                                     ? (struct commit *)a_obj
 -                                   : lookup_commit_reference(a_obj->sha1));
 +                                   : lookup_commit_reference(a_obj->oid.hash));
                                b = (b_obj->type == OBJ_COMMIT
                                     ? (struct commit *)b_obj
 -                                   : lookup_commit_reference(b_obj->sha1));
 +                                   : lookup_commit_reference(b_obj->oid.hash));
                                if (!a || !b)
                                        goto missing;
                                exclude = get_merge_bases(a, b);
@@@ -1993,10 -1998,10 +1995,10 @@@ static int handle_revision_opt(struct r
        } else if (!strcmp(arg, "--full-history")) {
                revs->simplify_history = 0;
        } else if (!strcmp(arg, "--relative-date")) {
 -              revs->date_mode = DATE_RELATIVE;
 +              revs->date_mode.type = DATE_RELATIVE;
                revs->date_mode_explicit = 1;
        } else if ((argcount = parse_long_opt("date", argv, &optarg))) {
 -              revs->date_mode = parse_date_format(optarg);
 +              parse_date_format(optarg, &revs->date_mode);
                revs->date_mode_explicit = 1;
                return argcount;
        } else if (!strcmp(arg, "--log-size")) {
@@@ -2073,23 -2078,14 +2075,23 @@@ void parse_revision_opt(struct rev_inf
        ctx->argc -= n;
  }
  
 +static int for_each_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data, const char *term) {
 +      struct strbuf bisect_refs = STRBUF_INIT;
 +      int status;
 +      strbuf_addf(&bisect_refs, "refs/bisect/%s", term);
 +      status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn, cb_data);
 +      strbuf_release(&bisect_refs);
 +      return status;
 +}
 +
  static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)
  {
 -      return for_each_ref_in_submodule(submodule, "refs/bisect/bad", fn, cb_data);
 +      return for_each_bisect_ref(submodule, fn, cb_data, term_bad);
  }
  
  static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)
  {
 -      return for_each_ref_in_submodule(submodule, "refs/bisect/good", fn, cb_data);
 +      return for_each_bisect_ref(submodule, fn, cb_data, term_good);
  }
  
  static int handle_revision_pseudo_opt(const char *submodule,
                handle_refs(submodule, revs, *flags, for_each_branch_ref_submodule);
                clear_ref_exclusion(&revs->ref_excludes);
        } else if (!strcmp(arg, "--bisect")) {
 +              read_bisect_terms(&term_bad, &term_good);
                handle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);
                handle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM), for_each_good_bisect_ref);
                revs->bisect = 1;
@@@ -2726,7 -2721,10 +2728,7 @@@ static void simplify_merges(struct rev_
                yet_to_do = NULL;
                tail = &yet_to_do;
                while (list) {
 -                      commit = list->item;
 -                      next = list->next;
 -                      free(list);
 -                      list = next;
 +                      commit = pop_commit(&list);
                        tail = simplify_one(revs, commit, tail);
                }
        }
        while (list) {
                struct merge_simplify_state *st;
  
 -              commit = list->item;
 -              next = list->next;
 -              free(list);
 -              list = next;
 +              commit = pop_commit(&list);
                st = locate_simplify_state(revs, commit);
                if (st->simplified == commit)
                        tail = &commit_list_insert(commit, tail)->next;
@@@ -2938,7 -2939,7 +2940,7 @@@ static int commit_match(struct commit *
        if (opt->show_notes) {
                if (!buf.len)
                        strbuf_addstr(&buf, message);
 -              format_display_notes(commit->object.sha1, &buf, encoding, 1);
 +              format_display_notes(commit->object.oid.hash, &buf, encoding, 1);
        }
  
        /*
@@@ -2968,7 -2969,7 +2970,7 @@@ enum commit_action get_commit_action(st
  {
        if (commit->object.flags & SHOWN)
                return commit_ignore;
 -      if (revs->unpacked && has_sha1_pack(commit->object.sha1))
 +      if (revs->unpacked && has_sha1_pack(commit->object.oid.hash))
                return commit_ignore;
        if (revs->show_all)
                return commit_show;
@@@ -3094,7 -3095,7 +3096,7 @@@ static void track_linear(struct rev_inf
                struct commit_list *p;
                for (p = revs->previous_parents; p; p = p->next)
                        if (p->item == NULL || /* first commit */
 -                          !hashcmp(p->item->object.sha1, commit->object.sha1))
 +                          !oidcmp(&p->item->object.oid, &commit->object.oid))
                                break;
                revs->linear = p != NULL;
        }
@@@ -3112,7 -3113,11 +3114,7 @@@ static struct commit *get_revision_1(st
                return NULL;
  
        do {
 -              struct commit_list *entry = revs->commits;
 -              struct commit *commit = entry->item;
 -
 -              revs->commits = entry->next;
 -              free(entry);
 +              struct commit *commit = pop_commit(&revs->commits);
  
                if (revs->reflog_info) {
                        save_parents(revs, commit);
                        if (add_parents_to_list(revs, commit, &revs->commits, NULL) < 0) {
                                if (!revs->ignore_missing_links)
                                        die("Failed to traverse parents of commit %s",
 -                                              sha1_to_hex(commit->object.sha1));
 +                                              oid_to_hex(&commit->object.oid));
                        }
                }
  
                        continue;
                case commit_error:
                        die("Failed to simplify parents of commit %s",
 -                          sha1_to_hex(commit->object.sha1));
 +                          oid_to_hex(&commit->object.oid));
                default:
                        if (revs->track_linear)
                                track_linear(revs, commit);