[PATCH v4 0/2] merge-tree: fix segmentation fault in read-only repositories

["Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>]

Turns out that the segmentation fault reported by Taylor
[https://lore.kernel.org/git/YyopQD+LvPucnz3w@nand.local/] happened while
testing merge-ort in a read-only repository, and that the upstream version
of git merge-tree is as affected as GitHub's internal version.

Changes since v3:

 * I now consistently use the pattern int ret = 0; ... if (...) ret = -1.
 * I added a commit to properly propagate write failures through the
   handle_content_merge() call path, even if it is not really critical (it
   just fails sooner, but the merge would have failed just the same without
   this patch).

Changes since v2:

 * Completely changed the approach by no longer touching
   builtin/merge-tree.c but instead fixing the underlying merge-ort layer:
   we no longer ignore the return value of write_tree() and
   write_object_file().

Changes since v1:

 * Using the SANITY prereq now
 * If the merge was aborted due to a write error, exit with a non-zero code
   even if the (potentially partial) merge is clean
 * The test case now also verifies that the git merge-tree command fails in
   a read-only repository even if the merge would have succeeded

Johannes Schindelin (2):
  merge-ort: fix segmentation fault in read-only repositories
  merge-ort: return early when failing to write a blob

 merge-ort.c                      | 94 ++++++++++++++++++++------------
 t/t4301-merge-tree-write-tree.sh |  9 +++
 2 files changed, 69 insertions(+), 34 deletions(-)


base-commit: dda7228a83e2e9ff584bf6adbf55910565b41e14
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1362%2Fdscho%2Fmerge-tree-in-read-only-repos-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1362/dscho/merge-tree-in-read-only-repos-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1362

Range-diff vs v3:

 1:  198ff455f90 ! 1:  ab6df092eba merge-ort: fix segmentation fault in read-only repositories
     @@ merge-ort.c: static int tree_entry_order(const void *a_, const void *b_)
       	unsigned int nr;
       	struct strbuf buf = STRBUF_INIT;
      -	int i;
     -+	int i, ret;
     ++	int i, ret = 0;
       
       	assert(offset <= versions->nr);
       	nr = versions->nr - offset;
     @@ merge-ort.c: 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);
     -+	ret = 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;
       }
     @@ merge-ort.c: 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;
     ++	int ret = 0;
       
       	trace2_region_enter("merge", "process_entries setup", opt->repo);
       	if (strmap_empty(&opt->priv->paths)) {
     @@ merge-ort.c: static void process_entries(struct merge_options *opt,
       	}
      -	write_tree(result_oid, &dir_metadata.versions, 0,
      -		   opt->repo->hash_algo->rawsz);
     -+	ret = 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);
 -:  ----------- > 2:  087207ae0b0 merge-ort: return early when failing to write a blob

-- 
gitgitgadget