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 <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Derrick Stolee committed
Jun 12, 2019 at 06:29 UTC
e103f7276f0d809c2935ebc1a3d68c6bbfaed23d
6 files changed
+77
-39
builtin/commit-graph.c
+10
-10
@@ -141,6 +141,7 @@ static int graph_write(int argc, const char **argv)
141
struct string_list *pack_indexes = NULL;
142
struct string_list *commit_hex = NULL;
143
struct string_list lines;
144
+ int result = 0;
145
146
static struct option builtin_commit_graph_write_options[] = {
147
OPT_STRING(0, "object-dir", &opts.obj_dir,
@@ -168,10 +169,8 @@ static int graph_write(int argc, const char **argv)
169
170
read_replace_refs = 0;
171
171
- if (opts.reachable) {
172
- write_commit_graph_reachable(opts.obj_dir, opts.append, 1);
173
- return 0;
174
- }
172
+ if (opts.reachable)
173
+ return write_commit_graph_reachable(opts.obj_dir, opts.append, 1);
174
175
string_list_init(&lines, 0);
176
if (opts.stdin_packs || opts.stdin_commits) {
@@ -188,14 +187,15 @@ static int graph_write(int argc, const char **argv)
187
UNLEAK(buf);
188
}
189
191
- write_commit_graph(opts.obj_dir,
192
- pack_indexes,
193
- commit_hex,
194
- opts.append,
195
- 1);
190
+ if (write_commit_graph(opts.obj_dir,
191
+ pack_indexes,
192
+ commit_hex,
193
+ opts.append,
194
+ 1))
195
+ result = 1;
196
197
UNLEAK(lines);
198
- return 0;
198
+ return result;
199
}
200
201
int cmd_commit_graph(int argc, const char **argv, const char *prefix)
builtin/commit.c
+3
-2
@@ -1669,8 +1669,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
1669
"new_index file. Check that disk is not full and quota is\n"
1670
"not exceeded, and then \"git reset HEAD\" to recover."));
1671
1672
- if (git_env_bool(GIT_TEST_COMMIT_GRAPH, 0))
1673
- write_commit_graph_reachable(get_object_directory(), 0, 0);
1672
+ if (git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&
1673
+ write_commit_graph_reachable(get_object_directory(), 0, 0))
1674
+ return 1;
1675
1676
repo_rerere(the_repository, 0);
1677
run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
builtin/gc.c
+4
-3
@@ -664,9 +664,10 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
664
clean_pack_garbage();
665
}
666
667
- if (gc_write_commit_graph)
668
- write_commit_graph_reachable(get_object_directory(), 0,
669
- !quiet && !daemonized);
667
+ if (gc_write_commit_graph &&
668
+ write_commit_graph_reachable(get_object_directory(), 0,
669
+ !quiet && !daemonized))
670
+ return 1;
671
672
if (auto_gc && too_many_loose_objects())
673
warning(_("There are too many unreachable loose objects; "
commit-graph.c
+41
-19
@@ -851,27 +851,30 @@ static int add_ref_to_list(const char *refname,
851
return 0;
852
}
853
854
-void write_commit_graph_reachable(const char *obj_dir, int append,
855
- int report_progress)
854
+int write_commit_graph_reachable(const char *obj_dir, int append,
855
+ int report_progress)
856
{
857
struct string_list list = STRING_LIST_INIT_DUP;
858
+ int result;
859
860
for_each_ref(add_ref_to_list, &list);
860
- write_commit_graph(obj_dir, NULL, &list, append, report_progress);
861
+ result = write_commit_graph(obj_dir, NULL, &list,
862
+ append, report_progress);
863
864
string_list_clear(&list, 0);
865
+ return result;
866
}
867
865
-void write_commit_graph(const char *obj_dir,
866
- struct string_list *pack_indexes,
867
- struct string_list *commit_hex,
868
- int append, int report_progress)
868
+int write_commit_graph(const char *obj_dir,
869
+ struct string_list *pack_indexes,
870
+ struct string_list *commit_hex,
871
+ int append, int report_progress)
872
{
873
struct packed_oid_list oids;
874
struct packed_commit_list commits;
875
struct hashfile *f;
876
uint32_t i, count_distinct = 0;
874
- char *graph_name;
877
+ char *graph_name = NULL;
878
struct lock_file lk = LOCK_INIT;
879
uint32_t chunk_ids[5];
880
uint64_t chunk_offsets[5];
@@ -883,15 +886,17 @@ void write_commit_graph(const char *obj_dir,
886
uint64_t progress_cnt = 0;
887
struct strbuf progress_title = STRBUF_INIT;
888
unsigned long approx_nr_objects;
889
+ int res = 0;
890
891
if (!commit_graph_compatible(the_repository))
888
- return;
892
+ return 0;
893
894
oids.nr = 0;
895
approx_nr_objects = approximate_object_count();
896
oids.alloc = approx_nr_objects / 32;
897
oids.progress = NULL;
898
oids.progress_done = 0;
899
+ commits.list = NULL;
900
901
if (append) {
902
prepare_commit_graph_one(the_repository, obj_dir);
@@ -932,10 +937,16 @@ void write_commit_graph(const char *obj_dir,
937
strbuf_setlen(&packname, dirlen);
938
strbuf_addstr(&packname, pack_indexes->items[i].string);
939
p = add_packed_git(packname.buf, packname.len, 1);
935
- if (!p)
936
- die(_("error adding pack %s"), packname.buf);
937
- if (open_pack_index(p))
938
- die(_("error opening index for %s"), packname.buf);
940
+ if (!p) {
941
+ error(_("error adding pack %s"), packname.buf);
942
+ res = -1;
943
+ goto cleanup;
944
+ }
945
+ if (open_pack_index(p)) {
946
+ error(_("error opening index for %s"), packname.buf);
947
+ res = -1;
948
+ goto cleanup;
949
+ }
950
for_each_object_in_pack(p, add_packed_commits, &oids,
951
FOR_EACH_OBJECT_PACK_ORDER);
952
close_pack(p);
@@ -1006,8 +1017,11 @@ void write_commit_graph(const char *obj_dir,
1017
}
1018
stop_progress(&progress);
1019
1009
- if (count_distinct >= GRAPH_EDGE_LAST_MASK)
1010
- die(_("the commit graph format cannot write %d commits"), count_distinct);
1020
+ if (count_distinct >= GRAPH_EDGE_LAST_MASK) {
1021
+ error(_("the commit graph format cannot write %d commits"), count_distinct);
1022
+ res = -1;
1023
+ goto cleanup;
1024
+ }
1025
1026
commits.nr = 0;
1027
commits.alloc = count_distinct;
@@ -1039,16 +1053,21 @@ void write_commit_graph(const char *obj_dir,
1053
num_chunks = num_extra_edges ? 4 : 3;
1054
stop_progress(&progress);
1055
1042
- if (commits.nr >= GRAPH_EDGE_LAST_MASK)
1043
- die(_("too many commits to write graph"));
1056
+ if (commits.nr >= GRAPH_EDGE_LAST_MASK) {
1057
+ error(_("too many commits to write graph"));
1058
+ res = -1;
1059
+ goto cleanup;
1060
+ }
1061
1062
compute_generation_numbers(&commits, report_progress);
1063
1064
graph_name = get_commit_graph_filename(obj_dir);
1065
if (safe_create_leading_directories(graph_name)) {
1066
UNLEAK(graph_name);
1050
- die_errno(_("unable to create leading directories of %s"),
1051
- graph_name);
1067
+ error(_("unable to create leading directories of %s"),
1068
+ graph_name);
1069
+ res = -1;
1070
+ goto cleanup;
1071
}
1072
1073
hold_lock_file_for_update(&lk, graph_name, LOCK_DIE_ON_ERROR);
@@ -1107,9 +1126,12 @@ void write_commit_graph(const char *obj_dir,
1126
finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_FSYNC);
1127
commit_lock_file(&lk);
1128
1129
+cleanup:
1130
free(graph_name);
1131
free(commits.list);
1132
free(oids.list);
1133
+
1134
+ return res;
1135
}
1136
1137
#define VERIFY_COMMIT_GRAPH_ERROR_HASH 2
commit-graph.h
+11
-5
@@ -65,12 +65,18 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd,
65
*/
66
int generation_numbers_enabled(struct repository *r);
67
68
-void write_commit_graph_reachable(const char *obj_dir, int append,
68
+/*
69
+ * The write_commit_graph* methods return zero on success
70
+ * and a negative value on failure. Note that if the repository
71
+ * is not compatible with the commit-graph feature, then the
72
+ * methods will return 0 without writing a commit-graph.
73
+ */
74
+int write_commit_graph_reachable(const char *obj_dir, int append,
75
int report_progress);
70
-void write_commit_graph(const char *obj_dir,
71
- struct string_list *pack_indexes,
72
- struct string_list *commit_hex,
73
- int append, int report_progress);
76
+int write_commit_graph(const char *obj_dir,
77
+ struct string_list *pack_indexes,
78
+ struct string_list *commit_hex,
79
+ int append, int report_progress);
80
81
int verify_commit_graph(struct repository *r, struct commit_graph *g);
82
t/t5318-commit-graph.sh
+8
@@ -23,6 +23,14 @@ test_expect_success 'write graph with no packs' '
23
test_path_is_file info/commit-graph
24
'
25
26
+test_expect_success 'close with correct error on bad input' '
27
+ cd "$TRASH_DIRECTORY/full" &&
28
+ echo doesnotexist >in &&
29
+ { git commit-graph write --stdin-packs <in 2>stderr; ret=$?; } &&
30
+ test "$ret" = 1 &&
31
+ test_i18ngrep "error adding pack" stderr
32
+'
33
+
34
test_expect_success 'create commits and repack' '
35
cd "$TRASH_DIRECTORY/full" &&
36
for i in $(test_seq 3)