Merge branch 'ds/commit-graph-leakfix'
authorJunio C Hamano <gitster@pobox.com>
Fri, 19 Oct 2018 04:34:07 +0000 (13:34 +0900)
committerJunio C Hamano <gitster@pobox.com>
Fri, 19 Oct 2018 04:34:07 +0000 (13:34 +0900)
Code clean-up.

* ds/commit-graph-leakfix:
commit-graph: reduce initial oid allocation
builtin/commit-graph.c: UNLEAK variables
commit-graph: clean up leaked memory during write

1  2 
builtin/commit-graph.c
commit-graph.c
diff --combined builtin/commit-graph.c
index 22b974f4b434913cf3ebe9e4073d2cb847f8a468,66f12eb009f1e41b6190ca23eb407c84b0f9380a..c02a3f1221d625ad6407317199cc35c7a56a09ba
@@@ -64,6 -64,7 +64,7 @@@ static int graph_verify(int argc, cons
        if (!graph)
                return 0;
  
+       UNLEAK(graph);
        return verify_commit_graph(the_repository, graph);
  }
  
@@@ -89,10 -90,8 +90,8 @@@ static int graph_read(int argc, const c
        graph_name = get_commit_graph_filename(opts.obj_dir);
        graph = load_commit_graph_one(graph_name);
  
-       if (!graph) {
-               UNLEAK(graph_name);
+       if (!graph)
                die("graph file %s does not exist", graph_name);
-       }
  
        FREE_AND_NULL(graph_name);
  
                printf(" large_edges");
        printf("\n");
  
-       free_commit_graph(graph);
+       UNLEAK(graph);
  
        return 0;
  }
  
 +extern int read_replace_refs;
 +
  static int graph_write(int argc, const char **argv)
  {
        struct string_list *pack_indexes = NULL;
        if (!opts.obj_dir)
                opts.obj_dir = get_object_directory();
  
 +      read_replace_refs = 0;
 +
        if (opts.reachable) {
                write_commit_graph_reachable(opts.obj_dir, opts.append, 1);
                return 0;
                        pack_indexes = &lines;
                if (opts.stdin_commits)
                        commit_hex = &lines;
+               UNLEAK(buf);
        }
  
        write_commit_graph(opts.obj_dir,
                           opts.append,
                           1);
  
-       string_list_clear(&lines, 0);
+       UNLEAK(lines);
        return 0;
  }
  
diff --combined commit-graph.c
index a6867586039b507e7802a469a1a3bed82e44bddb,e773703e1ddd6116abcd910caeb3cd72b862658c..40c855f1855595fbf5b285008d3c829e6a2f41eb
@@@ -13,8 -13,6 +13,8 @@@
  #include "commit-graph.h"
  #include "object-store.h"
  #include "alloc.h"
 +#include "hashmap.h"
 +#include "replace-object.h"
  #include "progress.h"
  
  #define GRAPH_SIGNATURE 0x43475048 /* "CGPH" */
@@@ -59,28 -57,6 +59,28 @@@ static struct commit_graph *alloc_commi
        return g;
  }
  
 +extern int read_replace_refs;
 +
 +static int commit_graph_compatible(struct repository *r)
 +{
 +      if (!r->gitdir)
 +              return 0;
 +
 +      if (read_replace_refs) {
 +              prepare_replace_object(r);
 +              if (hashmap_get_size(&r->objects->replace_map->map))
 +                      return 0;
 +      }
 +
 +      prepare_commit_graft(r);
 +      if (r->parsed_objects && r->parsed_objects->grafts_nr)
 +              return 0;
 +      if (is_repository_shallow(r))
 +              return 0;
 +
 +      return 1;
 +}
 +
  struct commit_graph *load_commit_graph_one(const char *graph_file)
  {
        void *graph_map;
@@@ -238,9 -214,8 +238,9 @@@ static int prepare_commit_graph(struct 
                return !!r->objects->commit_graph;
        r->objects->commit_graph_attempted = 1;
  
 -      if (repo_config_get_bool(r, "core.commitgraph", &config_value) ||
 -          !config_value)
 +      if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&
 +          (repo_config_get_bool(r, "core.commitgraph", &config_value) ||
 +          !config_value))
                /*
                 * This repository is not configured to use commit graphs, so
                 * do not load one. (But report commit_graph_attempted anyway
                 */
                return 0;
  
 +      if (!commit_graph_compatible(r))
 +              return 0;
 +
        obj_dir = r->objects->objectdir;
        prepare_commit_graph_one(r, obj_dir);
        prepare_alt_odb(r);
        return !!r->objects->commit_graph;
  }
  
 -static void close_commit_graph(void)
 +int generation_numbers_enabled(struct repository *r)
 +{
 +      uint32_t first_generation;
 +      struct commit_graph *g;
 +      if (!prepare_commit_graph(r))
 +             return 0;
 +
 +      g = r->objects->commit_graph;
 +
 +      if (!g->num_commits)
 +              return 0;
 +
 +      first_generation = get_be32(g->chunk_commit_data +
 +                                  g->hash_len + 8) >> 2;
 +
 +      return !!first_generation;
 +}
 +
 +void close_commit_graph(struct repository *r)
  {
 -      free_commit_graph(the_repository->objects->commit_graph);
 -      the_repository->objects->commit_graph = NULL;
 +      free_commit_graph(r->objects->commit_graph);
 +      r->objects->commit_graph = NULL;
  }
  
  static int bsearch_graph(struct commit_graph *g, struct object_id *oid, uint32_t *pos)
@@@ -739,11 -693,12 +739,12 @@@ static int add_ref_to_list(const char *
  void write_commit_graph_reachable(const char *obj_dir, int append,
                                  int report_progress)
  {
-       struct string_list list;
+       struct string_list list = STRING_LIST_INIT_DUP;
  
-       string_list_init(&list, 1);
        for_each_ref(add_ref_to_list, &list);
        write_commit_graph(obj_dir, NULL, &list, append, report_progress);
+       string_list_clear(&list, 0);
  }
  
  void write_commit_graph(const char *obj_dir,
        struct commit_list *parent;
        struct progress *progress = NULL;
  
 +      if (!commit_graph_compatible(the_repository))
 +              return;
 +
        oids.nr = 0;
-       oids.alloc = approximate_object_count() / 4;
+       oids.alloc = approximate_object_count() / 32;
        oids.progress = NULL;
        oids.progress_done = 0;
  
                                die(_("error opening index for %s"), packname.buf);
                        for_each_object_in_pack(p, add_packed_commits, &oids, 0);
                        close_pack(p);
+                       free(p);
                }
                stop_progress(&oids.progress);
                strbuf_release(&packname);
  
        count_distinct = 1;
        for (i = 1; i < oids.nr; i++) {
 -              if (oidcmp(&oids.list[i-1], &oids.list[i]))
 +              if (!oideq(&oids.list[i - 1], &oids.list[i]))
                        count_distinct++;
        }
  
        num_extra_edges = 0;
        for (i = 0; i < oids.nr; i++) {
                int num_parents = 0;
 -              if (i > 0 && !oidcmp(&oids.list[i-1], &oids.list[i]))
 +              if (i > 0 && oideq(&oids.list[i - 1], &oids.list[i]))
                        continue;
  
                commits.list[commits.nr] = lookup_commit(the_repository, &oids.list[i]);
        compute_generation_numbers(&commits, report_progress);
  
        graph_name = get_commit_graph_filename(obj_dir);
-       if (safe_create_leading_directories(graph_name))
+       if (safe_create_leading_directories(graph_name)) {
+               UNLEAK(graph_name);
                die_errno(_("unable to create leading directories of %s"),
                          graph_name);
+       }
  
        hold_lock_file_for_update(&lk, graph_name, LOCK_DIE_ON_ERROR);
        f = hashfd(lk.tempfile->fd, lk.tempfile->filename.buf);
        write_graph_chunk_data(f, GRAPH_OID_LEN, commits.list, commits.nr);
        write_graph_chunk_large_edges(f, commits.list, commits.nr);
  
 -      close_commit_graph();
 +      close_commit_graph(the_repository);
        finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_FSYNC);
        commit_lock_file(&lk);
  
+       free(graph_name);
+       free(commits.list);
        free(oids.list);
-       oids.alloc = 0;
-       oids.nr = 0;
  }
  
  #define VERIFY_COMMIT_GRAPH_ERROR_HASH 2
@@@ -994,7 -949,7 +998,7 @@@ int verify_commit_graph(struct reposito
        f = hashfd(devnull, NULL);
        hashwrite(f, g->data, g->data_len - g->hash_len);
        finalize_hashfile(f, checksum.hash, CSUM_CLOSE);
 -      if (hashcmp(checksum.hash, g->data + g->data_len - g->hash_len)) {
 +      if (!hasheq(checksum.hash, g->data + g->data_len - g->hash_len)) {
                graph_report(_("the commit-graph file has incorrect checksum and is likely corrupt"));
                verify_commit_graph_error = VERIFY_COMMIT_GRAPH_ERROR_HASH;
        }
                        continue;
                }
  
 -              if (oidcmp(&get_commit_tree_in_graph_one(g, graph_commit)->object.oid,
 +              if (!oideq(&get_commit_tree_in_graph_one(g, graph_commit)->object.oid,
                           get_commit_tree_oid(odb_commit)))
                        graph_report("root tree OID for commit %s in commit-graph is %s != %s",
                                     oid_to_hex(&cur_oid),
                                break;
                        }
  
 -                      if (oidcmp(&graph_parents->item->object.oid, &odb_parents->item->object.oid))
 +                      if (!oideq(&graph_parents->item->object.oid, &odb_parents->item->object.oid))
                                graph_report("commit-graph parent for %s is %s != %s",
                                             oid_to_hex(&cur_oid),
                                             oid_to_hex(&graph_parents->item->object.oid),