From: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: sandals@crustytoothpaste.net, avarab@gmail.com, peff@peff.net,
Junio C Hamano <gitster@pobox.com>,
Derrick Stolee <dstolee@microsoft.com>
Subject: [PATCH v5 02/11] commit-graph: return with errors during write
Date: Wed, 12 Jun 2019 06:29:37 -0700 (PDT) [thread overview]
Message-ID: <95f66e85b2fe93a218dad4c04c16718d053fb002.1560346173.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.112.v5.git.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
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 <dstolee@microsoft.com>
---
builtin/commit-graph.c | 20 +++++++-------
builtin/commit.c | 5 ++--
builtin/gc.c | 7 ++---
commit-graph.c | 60 ++++++++++++++++++++++++++++-------------
commit-graph.h | 16 +++++++----
t/t5318-commit-graph.sh | 8 ++++++
6 files changed, 77 insertions(+), 39 deletions(-)
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 537fdfd0f0..2a1c4d701f 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -141,6 +141,7 @@ static int graph_write(int argc, const char **argv)
struct string_list *pack_indexes = NULL;
struct string_list *commit_hex = NULL;
struct string_list lines;
+ int result = 0;
static struct option builtin_commit_graph_write_options[] = {
OPT_STRING(0, "object-dir", &opts.obj_dir,
@@ -168,10 +169,8 @@ static int graph_write(int argc, const char **argv)
read_replace_refs = 0;
- if (opts.reachable) {
- write_commit_graph_reachable(opts.obj_dir, opts.append, 1);
- return 0;
- }
+ if (opts.reachable)
+ return write_commit_graph_reachable(opts.obj_dir, opts.append, 1);
string_list_init(&lines, 0);
if (opts.stdin_packs || opts.stdin_commits) {
@@ -188,14 +187,15 @@ static int graph_write(int argc, const char **argv)
UNLEAK(buf);
}
- write_commit_graph(opts.obj_dir,
- pack_indexes,
- commit_hex,
- opts.append,
- 1);
+ if (write_commit_graph(opts.obj_dir,
+ pack_indexes,
+ commit_hex,
+ opts.append,
+ 1))
+ result = 1;
UNLEAK(lines);
- return 0;
+ return result;
}
int cmd_commit_graph(int argc, const char **argv, const char *prefix)
diff --git a/builtin/commit.c b/builtin/commit.c
index 2986553d5f..b9ea7222fa 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1669,8 +1669,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
"new_index file. Check that disk is not full and quota is\n"
"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);
+ if (git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&
+ write_commit_graph_reachable(get_object_directory(), 0, 0))
+ return 1;
repo_rerere(the_repository, 0);
run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
diff --git a/builtin/gc.c b/builtin/gc.c
index 020f725acc..3984addf73 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -664,9 +664,10 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
clean_pack_garbage();
}
- if (gc_write_commit_graph)
- write_commit_graph_reachable(get_object_directory(), 0,
- !quiet && !daemonized);
+ if (gc_write_commit_graph &&
+ write_commit_graph_reachable(get_object_directory(), 0,
+ !quiet && !daemonized))
+ return 1;
if (auto_gc && too_many_loose_objects())
warning(_("There are too many unreachable loose objects; "
diff --git a/commit-graph.c b/commit-graph.c
index 66865acbd7..1b58d1da14 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -851,27 +851,30 @@ static int add_ref_to_list(const char *refname,
return 0;
}
-void write_commit_graph_reachable(const char *obj_dir, int append,
- int report_progress)
+int write_commit_graph_reachable(const char *obj_dir, int append,
+ int report_progress)
{
struct string_list list = STRING_LIST_INIT_DUP;
+ int result;
for_each_ref(add_ref_to_list, &list);
- write_commit_graph(obj_dir, NULL, &list, append, report_progress);
+ result = write_commit_graph(obj_dir, NULL, &list,
+ append, report_progress);
string_list_clear(&list, 0);
+ return result;
}
-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)
{
struct packed_oid_list oids;
struct packed_commit_list commits;
struct hashfile *f;
uint32_t i, count_distinct = 0;
- char *graph_name;
+ char *graph_name = NULL;
struct lock_file lk = LOCK_INIT;
uint32_t chunk_ids[5];
uint64_t chunk_offsets[5];
@@ -883,15 +886,17 @@ void write_commit_graph(const char *obj_dir,
uint64_t progress_cnt = 0;
struct strbuf progress_title = STRBUF_INIT;
unsigned long approx_nr_objects;
+ int res = 0;
if (!commit_graph_compatible(the_repository))
- return;
+ return 0;
oids.nr = 0;
approx_nr_objects = approximate_object_count();
oids.alloc = approx_nr_objects / 32;
oids.progress = NULL;
oids.progress_done = 0;
+ commits.list = NULL;
if (append) {
prepare_commit_graph_one(the_repository, obj_dir);
@@ -932,10 +937,16 @@ void write_commit_graph(const char *obj_dir,
strbuf_setlen(&packname, dirlen);
strbuf_addstr(&packname, pack_indexes->items[i].string);
p = add_packed_git(packname.buf, packname.len, 1);
- if (!p)
- die(_("error adding pack %s"), packname.buf);
- if (open_pack_index(p))
- die(_("error opening index for %s"), packname.buf);
+ if (!p) {
+ error(_("error adding pack %s"), packname.buf);
+ res = -1;
+ goto cleanup;
+ }
+ if (open_pack_index(p)) {
+ error(_("error opening index for %s"), packname.buf);
+ res = -1;
+ goto cleanup;
+ }
for_each_object_in_pack(p, add_packed_commits, &oids,
FOR_EACH_OBJECT_PACK_ORDER);
close_pack(p);
@@ -1006,8 +1017,11 @@ void write_commit_graph(const char *obj_dir,
}
stop_progress(&progress);
- if (count_distinct >= GRAPH_EDGE_LAST_MASK)
- die(_("the commit graph format cannot write %d commits"), count_distinct);
+ if (count_distinct >= GRAPH_EDGE_LAST_MASK) {
+ error(_("the commit graph format cannot write %d commits"), count_distinct);
+ res = -1;
+ goto cleanup;
+ }
commits.nr = 0;
commits.alloc = count_distinct;
@@ -1039,16 +1053,21 @@ void write_commit_graph(const char *obj_dir,
num_chunks = num_extra_edges ? 4 : 3;
stop_progress(&progress);
- if (commits.nr >= GRAPH_EDGE_LAST_MASK)
- die(_("too many commits to write graph"));
+ if (commits.nr >= GRAPH_EDGE_LAST_MASK) {
+ error(_("too many commits to write graph"));
+ res = -1;
+ goto cleanup;
+ }
compute_generation_numbers(&commits, report_progress);
graph_name = get_commit_graph_filename(obj_dir);
if (safe_create_leading_directories(graph_name)) {
UNLEAK(graph_name);
- die_errno(_("unable to create leading directories of %s"),
- graph_name);
+ error(_("unable to create leading directories of %s"),
+ graph_name);
+ res = -1;
+ goto cleanup;
}
hold_lock_file_for_update(&lk, graph_name, LOCK_DIE_ON_ERROR);
@@ -1107,9 +1126,12 @@ void write_commit_graph(const char *obj_dir,
finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_FSYNC);
commit_lock_file(&lk);
+cleanup:
free(graph_name);
free(commits.list);
free(oids.list);
+
+ return res;
}
#define VERIFY_COMMIT_GRAPH_ERROR_HASH 2
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);
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index e80c1cac02..3b6fd0d728 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -23,6 +23,14 @@ test_expect_success 'write graph with no packs' '
test_path_is_file info/commit-graph
'
+test_expect_success 'close with correct error on bad input' '
+ cd "$TRASH_DIRECTORY/full" &&
+ echo doesnotexist >in &&
+ { git commit-graph write --stdin-packs <in 2>stderr; ret=$?; } &&
+ test "$ret" = 1 &&
+ test_i18ngrep "error adding pack" stderr
+'
+
test_expect_success 'create commits and repack' '
cd "$TRASH_DIRECTORY/full" &&
for i in $(test_seq 3)
--
gitgitgadget
next prev parent reply other threads:[~2019-06-12 13:29 UTC|newest]
Thread overview: 89+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-23 21:59 [PATCH 0/6] Create commit-graph file format v2 Derrick Stolee via GitGitGadget
2019-01-23 21:59 ` [PATCH 1/6] commit-graph: return with errors during write Derrick Stolee via GitGitGadget
2019-01-23 21:59 ` [PATCH 2/6] commit-graph: collapse parameters into flags Derrick Stolee via GitGitGadget
2019-01-23 21:59 ` [PATCH 3/6] commit-graph: create new version flags Derrick Stolee via GitGitGadget
2019-01-23 21:59 ` [PATCH 4/6] commit-graph: add --version=<n> option Derrick Stolee via GitGitGadget
2019-01-24 9:31 ` Ævar Arnfjörð Bjarmason
2019-01-23 21:59 ` [PATCH 5/6] commit-graph: implement file format version 2 Derrick Stolee via GitGitGadget
2019-01-23 23:56 ` Jonathan Tan
2019-01-24 9:40 ` Ævar Arnfjörð Bjarmason
2019-01-24 14:34 ` Derrick Stolee
2019-03-21 9:21 ` Ævar Arnfjörð Bjarmason
2019-01-23 21:59 ` [PATCH 6/6] commit-graph: test verifying a corrupt v2 header Derrick Stolee via GitGitGadget
2019-01-23 23:59 ` Jonathan Tan
2019-01-24 23:05 ` [PATCH 0/6] Create commit-graph file format v2 Junio C Hamano
2019-01-24 23:39 ` Junio C Hamano
2019-01-25 13:54 ` Derrick Stolee
2019-04-24 19:58 ` [PATCH v2 0/5] " Derrick Stolee via GitGitGadget
2019-04-24 19:58 ` [PATCH v2 1/5] commit-graph: return with errors during write Derrick Stolee via GitGitGadget
2019-04-24 19:58 ` [PATCH v2 2/5] commit-graph: collapse parameters into flags Derrick Stolee via GitGitGadget
2019-04-25 5:21 ` Junio C Hamano
2019-04-24 19:58 ` [PATCH v2 3/5] commit-graph: create new version flags Derrick Stolee via GitGitGadget
2019-04-25 5:29 ` Junio C Hamano
2019-04-25 11:09 ` Derrick Stolee
2019-04-25 21:31 ` Ævar Arnfjörð Bjarmason
2019-04-26 2:20 ` Junio C Hamano
2019-04-24 19:58 ` [PATCH v2 4/5] commit-graph: add --version=<n> option Derrick Stolee via GitGitGadget
2019-04-24 19:58 ` [PATCH v2 5/5] commit-graph: implement file format version 2 Derrick Stolee via GitGitGadget
2019-04-25 22:09 ` [PATCH v2 0/5] Create commit-graph file format v2 Ævar Arnfjörð Bjarmason
2019-04-26 2:28 ` Junio C Hamano
2019-04-26 8:33 ` Ævar Arnfjörð Bjarmason
2019-04-26 12:06 ` Derrick Stolee
2019-04-26 13:55 ` Ævar Arnfjörð Bjarmason
2019-04-27 12:57 ` Ævar Arnfjörð Bjarmason
2019-05-01 13:11 ` [PATCH v3 0/6] " Derrick Stolee via GitGitGadget
2019-05-01 13:11 ` [PATCH v3 1/6] commit-graph: return with errors during write Derrick Stolee via GitGitGadget
2019-05-01 14:46 ` Ævar Arnfjörð Bjarmason
2019-05-01 13:11 ` [PATCH v3 2/6] commit-graph: collapse parameters into flags Derrick Stolee via GitGitGadget
2019-05-01 13:11 ` [PATCH v3 3/6] commit-graph: create new version parameter Derrick Stolee via GitGitGadget
2019-05-01 13:11 ` [PATCH v3 4/6] commit-graph: add --version=<n> option Derrick Stolee via GitGitGadget
2019-05-01 13:11 ` [PATCH v3 5/6] commit-graph: implement file format version 2 Derrick Stolee via GitGitGadget
2019-05-01 19:12 ` Ævar Arnfjörð Bjarmason
2019-05-01 19:56 ` Derrick Stolee
2019-05-01 13:11 ` [PATCH v3 6/6] commit-graph: remove Future Work section Derrick Stolee via GitGitGadget
2019-05-01 14:58 ` Ævar Arnfjörð Bjarmason
2019-05-01 19:59 ` Derrick Stolee
2019-05-01 20:25 ` [PATCH v3 0/6] Create commit-graph file format v2 Ævar Arnfjörð Bjarmason
2019-05-02 13:26 ` Derrick Stolee
2019-05-02 18:02 ` Ævar Arnfjörð Bjarmason
2019-05-03 12:47 ` Derrick Stolee
2019-05-03 13:41 ` Ævar Arnfjörð Bjarmason
2019-05-06 8:27 ` Christian Couder
2019-05-06 13:47 ` Derrick Stolee
2019-05-03 14:16 ` SZEDER Gábor
2019-05-03 15:11 ` Derrick Stolee
2019-05-09 14:22 ` [PATCH v4 00/11] Commit-graph write refactor (was: Create commit-graph file format v2) Derrick Stolee via GitGitGadget
2019-05-09 14:22 ` [PATCH v4 01/11] commit-graph: fix the_repository reference Derrick Stolee via GitGitGadget
2019-05-13 2:56 ` Junio C Hamano
2019-05-09 14:22 ` [PATCH v4 02/11] commit-graph: return with errors during write Derrick Stolee via GitGitGadget
2019-05-13 3:13 ` Junio C Hamano
2019-05-13 11:04 ` Derrick Stolee
2019-05-13 11:22 ` Derrick Stolee
2019-05-09 14:22 ` [PATCH v4 03/11] commit-graph: collapse parameters into flags Derrick Stolee via GitGitGadget
2019-05-13 3:44 ` Junio C Hamano
2019-05-13 11:07 ` Derrick Stolee
2019-05-09 14:22 ` [PATCH v4 04/11] commit-graph: remove Future Work section Derrick Stolee via GitGitGadget
2019-05-09 14:22 ` [PATCH v4 05/11] commit-graph: create write_commit_graph_context Derrick Stolee via GitGitGadget
2019-05-09 14:22 ` [PATCH v4 06/11] commit-graph: extract fill_oids_from_packs() Derrick Stolee via GitGitGadget
2019-05-13 5:05 ` Junio C Hamano
2019-05-09 14:22 ` [PATCH v4 07/11] commit-graph: extract fill_oids_from_commit_hex() Derrick Stolee via GitGitGadget
2019-05-09 14:22 ` [PATCH v4 08/11] commit-graph: extract fill_oids_from_all_packs() Derrick Stolee via GitGitGadget
2019-05-09 14:22 ` [PATCH v4 09/11] commit-graph: extract count_distinct_commits() Derrick Stolee via GitGitGadget
2019-05-09 14:22 ` [PATCH v4 10/11] commit-graph: extract copy_oids_to_commits() Derrick Stolee via GitGitGadget
2019-05-09 14:22 ` [PATCH v4 11/11] commit-graph: extract write_commit_graph_file() Derrick Stolee via GitGitGadget
2019-05-13 5:09 ` Junio C Hamano
2019-05-09 17:58 ` [PATCH v4 00/11] Commit-graph write refactor (was: Create commit-graph file format v2) Josh Steadmon
2019-06-12 13:29 ` [PATCH v5 " Derrick Stolee via GitGitGadget
2019-06-12 13:29 ` [PATCH v5 01/11] commit-graph: fix the_repository reference Derrick Stolee via GitGitGadget
2019-06-12 13:29 ` Derrick Stolee via GitGitGadget [this message]
2019-06-29 17:23 ` [PATCH v5 02/11] commit-graph: return with errors during write SZEDER Gábor
2019-07-01 12:19 ` Derrick Stolee
2019-06-12 13:29 ` [PATCH v5 03/11] commit-graph: collapse parameters into flags Derrick Stolee via GitGitGadget
2019-06-12 13:29 ` [PATCH v5 04/11] commit-graph: remove Future Work section Derrick Stolee via GitGitGadget
2019-06-12 13:29 ` [PATCH v5 05/11] commit-graph: create write_commit_graph_context Derrick Stolee via GitGitGadget
2019-06-12 13:29 ` [PATCH v5 06/11] commit-graph: extract fill_oids_from_packs() Derrick Stolee via GitGitGadget
2019-06-12 13:29 ` [PATCH v5 08/11] commit-graph: extract fill_oids_from_all_packs() Derrick Stolee via GitGitGadget
2019-06-12 13:29 ` [PATCH v5 07/11] commit-graph: extract fill_oids_from_commit_hex() Derrick Stolee via GitGitGadget
2019-06-12 13:29 ` [PATCH v5 09/11] commit-graph: extract count_distinct_commits() Derrick Stolee via GitGitGadget
2019-06-12 13:29 ` [PATCH v5 10/11] commit-graph: extract copy_oids_to_commits() Derrick Stolee via GitGitGadget
2019-06-12 13:29 ` [PATCH v5 11/11] commit-graph: extract write_commit_graph_file() Derrick Stolee via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=95f66e85b2fe93a218dad4c04c16718d053fb002.1560346173.git.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=avarab@gmail.com \
--cc=dstolee@microsoft.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
--cc=sandals@crustytoothpaste.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).