From e103f7276f0d809c2935ebc1a3d68c6bbfaed23d Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Wed, 12 Jun 2019 06:29:37 -0700 Subject: commit-graph: return with errors during write The write_commit_graph() method uses die() to report failure and exit when confronted with an unexpected condition. This use of die() in a library function is incorrect and is now replaced by error() statements and an int return type. Return zero on success and a negative value on failure. Now that we use 'goto cleanup' to jump to the terminal condition on an error, we have new paths that could lead to uninitialized values. New initializers are added to correct for this. The builtins 'commit-graph', 'gc', and 'commit' call these methods, so update them to check the return value. Test that 'git commit-graph write' returns a proper error code when hitting a failure condition in write_commit_graph(). Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- commit-graph.h | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'commit-graph.h') diff --git a/commit-graph.h b/commit-graph.h index 7dfb8c896f..869717ca19 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -65,12 +65,18 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd, */ int generation_numbers_enabled(struct repository *r); -void write_commit_graph_reachable(const char *obj_dir, int append, +/* + * The write_commit_graph* methods return zero on success + * and a negative value on failure. Note that if the repository + * is not compatible with the commit-graph feature, then the + * methods will return 0 without writing a commit-graph. + */ +int write_commit_graph_reachable(const char *obj_dir, int append, int report_progress); -void write_commit_graph(const char *obj_dir, - struct string_list *pack_indexes, - struct string_list *commit_hex, - int append, int report_progress); +int write_commit_graph(const char *obj_dir, + struct string_list *pack_indexes, + struct string_list *commit_hex, + int append, int report_progress); int verify_commit_graph(struct repository *r, struct commit_graph *g); -- cgit v1.2.3 From 5af803945212af875670582ff153ee05ec368b83 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Wed, 12 Jun 2019 06:29:38 -0700 Subject: commit-graph: collapse parameters into flags The write_commit_graph() and write_commit_graph_reachable() methods currently take two boolean parameters: 'append' and 'report_progress'. As we update these methods, adding more parameters this way becomes cluttered and hard to maintain. Collapse these parameters into a 'flags' parameter, and adjust the callers to provide flags as necessary. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/commit-graph.c | 8 +++++--- builtin/commit.c | 2 +- builtin/gc.c | 4 ++-- commit-graph.c | 9 +++++---- commit-graph.h | 8 +++++--- 5 files changed, 18 insertions(+), 13 deletions(-) (limited to 'commit-graph.h') diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 2a1c4d701f..d8efa5bab2 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -142,6 +142,7 @@ static int graph_write(int argc, const char **argv) struct string_list *commit_hex = NULL; struct string_list lines; int result = 0; + unsigned int flags = COMMIT_GRAPH_PROGRESS; static struct option builtin_commit_graph_write_options[] = { OPT_STRING(0, "object-dir", &opts.obj_dir, @@ -166,11 +167,13 @@ static int graph_write(int argc, const char **argv) die(_("use at most one of --reachable, --stdin-commits, or --stdin-packs")); if (!opts.obj_dir) opts.obj_dir = get_object_directory(); + if (opts.append) + flags |= COMMIT_GRAPH_APPEND; read_replace_refs = 0; if (opts.reachable) - return write_commit_graph_reachable(opts.obj_dir, opts.append, 1); + return write_commit_graph_reachable(opts.obj_dir, flags); string_list_init(&lines, 0); if (opts.stdin_packs || opts.stdin_commits) { @@ -190,8 +193,7 @@ static int graph_write(int argc, const char **argv) if (write_commit_graph(opts.obj_dir, pack_indexes, commit_hex, - opts.append, - 1)) + flags)) result = 1; UNLEAK(lines); diff --git a/builtin/commit.c b/builtin/commit.c index b9ea7222fa..b001ef565d 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1670,7 +1670,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) "not exceeded, and then \"git reset HEAD\" to recover.")); if (git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) && - write_commit_graph_reachable(get_object_directory(), 0, 0)) + write_commit_graph_reachable(get_object_directory(), 0)) return 1; repo_rerere(the_repository, 0); diff --git a/builtin/gc.c b/builtin/gc.c index 3984addf73..df2573f124 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -665,8 +665,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix) } if (gc_write_commit_graph && - write_commit_graph_reachable(get_object_directory(), 0, - !quiet && !daemonized)) + write_commit_graph_reachable(get_object_directory(), + !quiet && !daemonized ? COMMIT_GRAPH_PROGRESS : 0)) return 1; if (auto_gc && too_many_loose_objects()) diff --git a/commit-graph.c b/commit-graph.c index 1b58d1da14..fc40b531af 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -851,15 +851,14 @@ static int add_ref_to_list(const char *refname, return 0; } -int write_commit_graph_reachable(const char *obj_dir, int append, - int report_progress) +int write_commit_graph_reachable(const char *obj_dir, unsigned int flags) { struct string_list list = STRING_LIST_INIT_DUP; int result; for_each_ref(add_ref_to_list, &list); result = write_commit_graph(obj_dir, NULL, &list, - append, report_progress); + flags); string_list_clear(&list, 0); return result; @@ -868,7 +867,7 @@ int write_commit_graph_reachable(const char *obj_dir, int append, int write_commit_graph(const char *obj_dir, struct string_list *pack_indexes, struct string_list *commit_hex, - int append, int report_progress) + unsigned int flags) { struct packed_oid_list oids; struct packed_commit_list commits; @@ -887,6 +886,8 @@ int write_commit_graph(const char *obj_dir, struct strbuf progress_title = STRBUF_INIT; unsigned long approx_nr_objects; int res = 0; + int append = flags & COMMIT_GRAPH_APPEND; + int report_progress = flags & COMMIT_GRAPH_PROGRESS; if (!commit_graph_compatible(the_repository)) return 0; diff --git a/commit-graph.h b/commit-graph.h index 869717ca19..01538b5cf5 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -65,18 +65,20 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd, */ int generation_numbers_enabled(struct repository *r); +#define COMMIT_GRAPH_APPEND (1 << 0) +#define COMMIT_GRAPH_PROGRESS (1 << 1) + /* * The write_commit_graph* methods return zero on success * and a negative value on failure. Note that if the repository * is not compatible with the commit-graph feature, then the * methods will return 0 without writing a commit-graph. */ -int write_commit_graph_reachable(const char *obj_dir, int append, - int report_progress); +int write_commit_graph_reachable(const char *obj_dir, unsigned int flags); int write_commit_graph(const char *obj_dir, struct string_list *pack_indexes, struct string_list *commit_hex, - int append, int report_progress); + unsigned int flags); int verify_commit_graph(struct repository *r, struct commit_graph *g); -- cgit v1.2.3 From c3a3a964b29221a9b5fa305a08037c90b9f74be0 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Fri, 17 May 2019 11:41:47 -0700 Subject: commit-graph: use raw_object_store when closing The close_commit_graph() method took a repository struct, but then only uses the raw_object_store within. Change the function prototype to make the method more flexible. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- commit-graph.c | 8 ++++---- commit-graph.h | 2 +- upload-pack.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) (limited to 'commit-graph.h') diff --git a/commit-graph.c b/commit-graph.c index 9d2c72f5b4..76d189de45 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -359,10 +359,10 @@ int generation_numbers_enabled(struct repository *r) return !!first_generation; } -void close_commit_graph(struct repository *r) +void close_commit_graph(struct raw_object_store *o) { - free_commit_graph(r->objects->commit_graph); - r->objects->commit_graph = NULL; + free_commit_graph(o->commit_graph); + o->commit_graph = NULL; } static int bsearch_graph(struct commit_graph *g, struct object_id *oid, uint32_t *pos) @@ -1086,7 +1086,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) stop_progress(&ctx->progress); strbuf_release(&progress_title); - close_commit_graph(ctx->r); + close_commit_graph(ctx->r->objects); finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_FSYNC); commit_lock_file(&lk); diff --git a/commit-graph.h b/commit-graph.h index 01538b5cf5..390c7f6961 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -82,7 +82,7 @@ int write_commit_graph(const char *obj_dir, int verify_commit_graph(struct repository *r, struct commit_graph *g); -void close_commit_graph(struct repository *); +void close_commit_graph(struct raw_object_store *); void free_commit_graph(struct commit_graph *); #endif diff --git a/upload-pack.c b/upload-pack.c index d098ef5982..b51bed21e4 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -717,7 +717,7 @@ static void deepen_by_rev_list(struct packet_writer *writer, int ac, { struct commit_list *result; - close_commit_graph(the_repository); + close_commit_graph(the_repository->objects); result = get_shallow_commits_by_rev_list(ac, av, SHALLOW, NOT_SHALLOW); send_shallow(writer, result); free_commit_list(result); -- cgit v1.2.3