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
This commit is contained in:
Junio C Hamano 2022-10-25 17:11:34 -07:00
commit bcf22f29df
2 changed files with 69 additions and 34 deletions

View file

@ -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 @@ static void merge_ort_nonrecursive_internal(struct merge_options *opt,
}
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;

View file

@ -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