diff options
author | Junio C Hamano <gitster@pobox.com> | 2022-10-26 03:11:34 +0300 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2022-10-26 03:11:34 +0300 |
commit | bcf22f29df87f25582de0ac9274d1567fa16be6f (patch) | |
tree | 170b4e950ba1b15dc3d7b48f13e75211167c7b8c | |
parent | 7f8a6caee563de5b1df31f698dfbe4b55a6b2bea (diff) | |
parent | 92481d1b26ab57525f5efe01d01c7a3d337b8df7 (diff) |
Merge branch 'js/merge-ort-in-read-only-repo' into maint-2.38
In read-only repositories, "git merge-tree" tried to come up with a
merge result tree object, which it failed (which is not wrong) and
led to a segfault (which is bad), which has been corrected.
* js/merge-ort-in-read-only-repo:
merge-ort: return early when failing to write a blob
merge-ort: fix segmentation fault in read-only repositories
-rw-r--r-- | merge-ort.c | 94 | ||||
-rwxr-xr-x | t/t4301-merge-tree-write-tree.sh | 9 |
2 files changed, 69 insertions, 34 deletions
diff --git a/merge-ort.c b/merge-ort.c index 99dcee2db8a..e5f41cce481 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -2807,6 +2807,8 @@ static int process_renames(struct merge_options *opt, pathnames, 1 + 2 * opt->priv->call_depth, &merged); + if (clean_merge < 0) + return -1; if (!clean_merge && merged.mode == side1->stages[1].mode && oideq(&merged.oid, &side1->stages[1].oid)) @@ -2916,7 +2918,7 @@ static int process_renames(struct merge_options *opt, struct version_info merged; struct conflict_info *base, *side1, *side2; - unsigned clean; + int clean; pathnames[0] = oldpath; pathnames[other_source_index] = oldpath; @@ -2937,6 +2939,8 @@ static int process_renames(struct merge_options *opt, pathnames, 1 + 2 * opt->priv->call_depth, &merged); + if (clean < 0) + return -1; memcpy(&newinfo->stages[target_index], &merged, sizeof(merged)); @@ -3571,15 +3575,15 @@ static int tree_entry_order(const void *a_, const void *b_) b->string, strlen(b->string), bmi->result.mode); } -static void write_tree(struct object_id *result_oid, - struct string_list *versions, - unsigned int offset, - size_t hash_size) +static int write_tree(struct object_id *result_oid, + struct string_list *versions, + unsigned int offset, + size_t hash_size) { size_t maxlen = 0, extra; unsigned int nr; struct strbuf buf = STRBUF_INIT; - int i; + int i, ret = 0; assert(offset <= versions->nr); nr = versions->nr - offset; @@ -3605,8 +3609,10 @@ static void write_tree(struct object_id *result_oid, } /* Write this object file out, and record in result_oid */ - write_object_file(buf.buf, buf.len, OBJ_TREE, result_oid); + if (write_object_file(buf.buf, buf.len, OBJ_TREE, result_oid)) + ret = -1; strbuf_release(&buf); + return ret; } static void record_entry_for_tree(struct directory_versions *dir_metadata, @@ -3625,13 +3631,13 @@ static void record_entry_for_tree(struct directory_versions *dir_metadata, basename)->util = &mi->result; } -static void write_completed_directory(struct merge_options *opt, - const char *new_directory_name, - struct directory_versions *info) +static int write_completed_directory(struct merge_options *opt, + const char *new_directory_name, + struct directory_versions *info) { const char *prev_dir; struct merged_info *dir_info = NULL; - unsigned int offset; + unsigned int offset, ret = 0; /* * Some explanation of info->versions and info->offsets... @@ -3717,7 +3723,7 @@ static void write_completed_directory(struct merge_options *opt, * strcmp here.) */ if (new_directory_name == info->last_directory) - return; + return 0; /* * If we are just starting (last_directory is NULL), or last_directory @@ -3739,7 +3745,7 @@ static void write_completed_directory(struct merge_options *opt, */ string_list_append(&info->offsets, info->last_directory)->util = (void*)offset; - return; + return 0; } /* @@ -3769,8 +3775,9 @@ static void write_completed_directory(struct merge_options *opt, */ dir_info->is_null = 0; dir_info->result.mode = S_IFDIR; - write_tree(&dir_info->result.oid, &info->versions, offset, - opt->repo->hash_algo->rawsz); + if (write_tree(&dir_info->result.oid, &info->versions, offset, + opt->repo->hash_algo->rawsz) < 0) + ret = -1; } /* @@ -3798,13 +3805,15 @@ static void write_completed_directory(struct merge_options *opt, /* And, of course, we need to update last_directory to match. */ info->last_directory = new_directory_name; info->last_directory_len = strlen(info->last_directory); + + return ret; } /* Per entry merge function */ -static void process_entry(struct merge_options *opt, - const char *path, - struct conflict_info *ci, - struct directory_versions *dir_metadata) +static int process_entry(struct merge_options *opt, + const char *path, + struct conflict_info *ci, + struct directory_versions *dir_metadata) { int df_file_index = 0; @@ -3818,7 +3827,7 @@ static void process_entry(struct merge_options *opt, record_entry_for_tree(dir_metadata, path, &ci->merged); if (ci->filemask == 0) /* nothing else to handle */ - return; + return 0; assert(ci->df_conflict); } @@ -3865,7 +3874,7 @@ static void process_entry(struct merge_options *opt, */ if (ci->filemask == 1) { ci->filemask = 0; - return; + return 0; } /* @@ -4060,7 +4069,7 @@ static void process_entry(struct merge_options *opt, } else if (ci->filemask >= 6) { /* Need a two-way or three-way content merge */ struct version_info merged_file; - unsigned clean_merge; + int clean_merge; struct version_info *o = &ci->stages[0]; struct version_info *a = &ci->stages[1]; struct version_info *b = &ci->stages[2]; @@ -4069,6 +4078,8 @@ static void process_entry(struct merge_options *opt, ci->pathnames, opt->priv->call_depth * 2, &merged_file); + if (clean_merge < 0) + return -1; ci->merged.clean = clean_merge && !ci->df_conflict && !ci->path_conflict; ci->merged.result.mode = merged_file.mode; @@ -4164,6 +4175,7 @@ static void process_entry(struct merge_options *opt, /* Record metadata for ci->merged in dir_metadata */ record_entry_for_tree(dir_metadata, path, &ci->merged); + return 0; } static void prefetch_for_content_merges(struct merge_options *opt, @@ -4214,8 +4226,8 @@ static void prefetch_for_content_merges(struct merge_options *opt, oid_array_clear(&to_fetch); } -static void process_entries(struct merge_options *opt, - struct object_id *result_oid) +static int process_entries(struct merge_options *opt, + struct object_id *result_oid) { struct hashmap_iter iter; struct strmap_entry *e; @@ -4224,11 +4236,12 @@ static void process_entries(struct merge_options *opt, struct directory_versions dir_metadata = { STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP, NULL, 0 }; + int ret = 0; trace2_region_enter("merge", "process_entries setup", opt->repo); if (strmap_empty(&opt->priv->paths)) { oidcpy(result_oid, opt->repo->hash_algo->empty_tree); - return; + return 0; } /* Hack to pre-allocate plist to the desired size */ @@ -4270,13 +4283,19 @@ static void process_entries(struct merge_options *opt, */ struct merged_info *mi = entry->util; - write_completed_directory(opt, mi->directory_name, - &dir_metadata); + if (write_completed_directory(opt, mi->directory_name, + &dir_metadata) < 0) { + ret = -1; + goto cleanup; + } if (mi->clean) record_entry_for_tree(&dir_metadata, path, mi); else { struct conflict_info *ci = (struct conflict_info *)mi; - process_entry(opt, path, ci, &dir_metadata); + if (process_entry(opt, path, ci, &dir_metadata) < 0) { + ret = -1; + goto cleanup; + }; } } trace2_region_leave("merge", "processing", opt->repo); @@ -4291,12 +4310,16 @@ static void process_entries(struct merge_options *opt, fflush(stdout); BUG("dir_metadata accounting completely off; shouldn't happen"); } - write_tree(result_oid, &dir_metadata.versions, 0, - opt->repo->hash_algo->rawsz); + if (write_tree(result_oid, &dir_metadata.versions, 0, + opt->repo->hash_algo->rawsz) < 0) + ret = -1; +cleanup: string_list_clear(&plist, 0); string_list_clear(&dir_metadata.versions, 0); string_list_clear(&dir_metadata.offsets, 0); trace2_region_leave("merge", "process_entries cleanup", opt->repo); + + return ret; } /*** Function Grouping: functions related to merge_switch_to_result() ***/ @@ -4928,15 +4951,18 @@ redo: } trace2_region_enter("merge", "process_entries", opt->repo); - process_entries(opt, &working_tree_oid); + if (process_entries(opt, &working_tree_oid) < 0) + result->clean = -1; trace2_region_leave("merge", "process_entries", opt->repo); /* Set return values */ result->path_messages = &opt->priv->conflicts; - result->tree = parse_tree_indirect(&working_tree_oid); - /* existence of conflicted entries implies unclean */ - result->clean &= strmap_empty(&opt->priv->conflicted); + if (result->clean >= 0) { + result->tree = parse_tree_indirect(&working_tree_oid); + /* existence of conflicted entries implies unclean */ + result->clean &= strmap_empty(&opt->priv->conflicted); + } if (!opt->priv->call_depth) { result->priv = opt->priv; result->_properly_initialized = RESULT_INITIALIZED; diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh index 28ca5c38bb5..013b77144bd 100755 --- a/t/t4301-merge-tree-write-tree.sh +++ b/t/t4301-merge-tree-write-tree.sh @@ -810,4 +810,13 @@ test_expect_success 'can override merge of unrelated histories' ' test_cmp expect actual ' +test_expect_success SANITY 'merge-ort fails gracefully in a read-only repository' ' + git init --bare read-only && + git push read-only side1 side2 side3 && + test_when_finished "chmod -R u+w read-only" && + chmod -R a-w read-only && + test_must_fail git -C read-only merge-tree side1 side3 && + test_must_fail git -C read-only merge-tree side1 side2 +' + test_done |