diff --git a/Documentation/technical/api-merge.adoc b/Documentation/technical/api-merge.adoc index c2ba01828c..b691599393 100644 --- a/Documentation/technical/api-merge.adoc +++ b/Documentation/technical/api-merge.adoc @@ -20,11 +20,10 @@ responsible for a few things. Data structures --------------- -* `mmbuffer_t`, `mmfile_t` +* `mmfile_t` -These store data usable for use by the xdiff backend, for writing and -for reading, respectively. See `xdiff/xdiff.h` for the definitions -and `diff.c` for examples. +This stores a buffer and its size for input to or output from the xdiff +backend. See `xdiff/xdiff.h` for the definition and `diff.c` for examples. * `struct ll_merge_options` diff --git a/apply.c b/apply.c index 367271b8ac..a0b7119c63 100644 --- a/apply.c +++ b/apply.c @@ -3647,7 +3647,7 @@ static int three_way_merge(struct apply_state *state, { mmfile_t base_file, our_file, their_file; struct ll_merge_options merge_opts = LL_MERGE_OPTIONS_INIT; - mmbuffer_t result = { NULL }; + mmfile_t result = { NULL }; enum ll_merge_result status; /* resolve trivial cases first */ diff --git a/builtin/checkout.c b/builtin/checkout.c index c0f0d2c700..2d575a4f57 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -320,7 +320,7 @@ static int checkout_merged(int pos, const struct checkout *state, enum ll_merge_result merge_status; int status; struct object_id oid; - mmbuffer_t result_buf; + mmfile_t result_buf; struct object_id threeway[3]; unsigned mode = 0; struct ll_merge_options ll_opts = LL_MERGE_OPTIONS_INIT; diff --git a/builtin/merge-file.c b/builtin/merge-file.c index 8fa5765239..ddca408c46 100644 --- a/builtin/merge-file.c +++ b/builtin/merge-file.c @@ -64,7 +64,7 @@ int cmd_merge_file(int argc, { const char *names[3] = { 0 }; mmfile_t mmfs[3] = { 0 }; - mmbuffer_t result = { 0 }; + mmfile_t result = { 0 }; xmparam_t xmp = { 0 }; int ret = 0, i = 0, to_stdout = 0, object_id = 0; int quiet = 0; diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c index 49f41e520f..552c2ad736 100644 --- a/builtin/merge-tree.c +++ b/builtin/merge-tree.c @@ -109,7 +109,7 @@ static void *origin(struct merge_list *entry, size_t *size) return NULL; } -static int show_outf(void *priv UNUSED, mmbuffer_t *mb, int nbuf) +static int show_outf(void *priv UNUSED, mmfile_t *mb, int nbuf) { int i; for (i = 0; i < nbuf; i++) diff --git a/builtin/rerere.c b/builtin/rerere.c index a056cb791b..ef03b79f5b 100644 --- a/builtin/rerere.c +++ b/builtin/rerere.c @@ -16,7 +16,7 @@ static const char * const rerere_usage[] = { NULL, }; -static int outf(void *dummy UNUSED, mmbuffer_t *ptr, int nbuf) +static int outf(void *dummy UNUSED, mmfile_t *ptr, int nbuf) { int i; for (i = 0; i < nbuf; i++) @@ -34,8 +34,12 @@ static int diff_two(const char *file1, const char *label1, mmfile_t minus, plus; int ret; - if (read_mmfile(&minus, file1) || read_mmfile(&plus, file2)) + if (read_mmfile(&minus, file1)) return -1; + if (read_mmfile(&plus, file2)) { + free(minus.ptr); + return -1; + } printf("--- a/%s\n+++ b/%s\n", label1, label2); fflush(stdout); diff --git a/diff.c b/diff.c index 414532d09f..b4ac17f8ef 100644 --- a/diff.c +++ b/diff.c @@ -3646,7 +3646,7 @@ static void emit_binary_diff_body(struct diff_options *o, data = delta; data_size = delta_size; } else { - char *s = xstrfmt("%lu", two->size); + char *s = xstrfmt("%"PRIuMAX, (uintmax_t)two->size); emit_diff_symbol(o, DIFF_SYMBOL_BINARY_DIFF_HEADER_LITERAL, s, strlen(s), 0); free(s); diff --git a/merge-blobs.c b/merge-blobs.c index 16a75bd1e3..49dbec9529 100644 --- a/merge-blobs.c +++ b/merge-blobs.c @@ -38,7 +38,7 @@ static void *three_way_filemerge(struct index_state *istate, size_t *size) { enum ll_merge_result merge_status; - mmbuffer_t res; + mmfile_t res; /* * This function is only used by cmd_merge_tree, which diff --git a/merge-ll.c b/merge-ll.c index 0eadbfba23..996a38113f 100644 --- a/merge-ll.c +++ b/merge-ll.c @@ -22,7 +22,7 @@ struct ll_merge_driver; typedef enum ll_merge_result (*ll_merge_fn)(const struct ll_merge_driver *, - mmbuffer_t *result, + mmfile_t *result, const char *path, mmfile_t *orig, const char *orig_name, mmfile_t *src1, const char *name1, @@ -57,7 +57,7 @@ void reset_merge_attributes(void) * Built-in low-levels */ static enum ll_merge_result ll_binary_merge(const struct ll_merge_driver *drv UNUSED, - mmbuffer_t *result, + mmfile_t *result, const char *path UNUSED, mmfile_t *orig, const char *orig_name UNUSED, mmfile_t *src1, const char *name1 UNUSED, @@ -102,7 +102,7 @@ static enum ll_merge_result ll_binary_merge(const struct ll_merge_driver *drv UN } static enum ll_merge_result ll_xdl_merge(const struct ll_merge_driver *drv_unused, - mmbuffer_t *result, + mmfile_t *result, const char *path, mmfile_t *orig, const char *orig_name, mmfile_t *src1, const char *name1, @@ -148,7 +148,7 @@ static enum ll_merge_result ll_xdl_merge(const struct ll_merge_driver *drv_unuse } static enum ll_merge_result ll_union_merge(const struct ll_merge_driver *drv_unused, - mmbuffer_t *result, + mmfile_t *result, const char *path, mmfile_t *orig, const char *orig_name, mmfile_t *src1, const char *name1, @@ -203,7 +203,7 @@ static const char *temp_path_basename(struct tempfile *t) * User defined low-level merge driver support. */ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn, - mmbuffer_t *result, + mmfile_t *result, const char *path, mmfile_t *orig, const char *orig_name, mmfile_t *src1, const char *name1, @@ -215,8 +215,7 @@ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn, struct strbuf cmd = STRBUF_INIT; const char *format = fn->cmdline; struct child_process child = CHILD_PROCESS_INIT; - int status, fd; - struct stat st; + int status; enum ll_merge_result ret; assert(opts); @@ -255,24 +254,6 @@ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn, child.use_shell = 1; strvec_push(&child.args, cmd.buf); status = run_command(&child); - fd = open(get_tempfile_path(tmp_a), O_RDONLY); - if (fd < 0) - goto bad; - if (fstat(fd, &st)) - goto close_bad; - result->size = st.st_size; - result->ptr = xmallocz(result->size); - if (read_in_full(fd, result->ptr, result->size) != result->size) { - FREE_AND_NULL(result->ptr); - result->size = 0; - } - close_bad: - close(fd); - bad: - delete_tempfile(&tmp_o); - delete_tempfile(&tmp_a); - delete_tempfile(&tmp_b); - strbuf_release(&cmd); if (!status) ret = LL_MERGE_OK; else if (status <= 128) @@ -280,6 +261,14 @@ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn, else /* died due to a signal: WTERMSIG(status) + 128 */ ret = LL_MERGE_ERROR; + + if (read_mmfile(result, temp_path_basename(tmp_a)) < 0) + ret = LL_MERGE_ERROR; + + delete_tempfile(&tmp_o); + delete_tempfile(&tmp_a); + delete_tempfile(&tmp_b); + strbuf_release(&cmd); return ret; } @@ -418,7 +407,7 @@ static void normalize_file(mmfile_t *mm, const char *path, struct index_state *i } } -enum ll_merge_result ll_merge(mmbuffer_t *result_buf, +enum ll_merge_result ll_merge(mmfile_t *result_buf, const char *path, mmfile_t *ancestor, const char *ancestor_label, mmfile_t *ours, const char *our_label, diff --git a/merge-ll.h b/merge-ll.h index f26aef238d..f95332c682 100644 --- a/merge-ll.h +++ b/merge-ll.h @@ -16,7 +16,7 @@ * If you have no special requests, skip this and pass `NULL` * as the `opts` parameter to use the default options. * - * - Allocate an mmbuffer_t variable for the result. + * - Allocate an mmfile_t variable for the result. * * - Allocate and fill variables with the file's original content * and two modified versions (using `read_mmfile`, for example). @@ -100,7 +100,7 @@ enum ll_merge_result { * `.gitattributes` or `.git/info/attributes` into account. * Returns 0 for a clean merge. */ -enum ll_merge_result ll_merge(mmbuffer_t *result_buf, +enum ll_merge_result ll_merge(mmfile_t *result_buf, const char *path, mmfile_t *ancestor, const char *ancestor_label, mmfile_t *ours, const char *our_label, diff --git a/merge-ort.c b/merge-ort.c index c410a5d353..1d3d193d35 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -2111,7 +2111,7 @@ static int merge_3way(struct merge_options *opt, const struct object_id *b, const char *pathnames[3], const int extra_marker_size, - mmbuffer_t *result_buf) + mmfile_t *result_buf) { mmfile_t orig, src1, src2; struct ll_merge_options ll_opts = LL_MERGE_OPTIONS_INIT; @@ -2247,7 +2247,7 @@ static int handle_content_merge(struct merge_options *opt, /* Remaining rules depend on file vs. submodule vs. symlink. */ else if (S_ISREG(a->mode)) { - mmbuffer_t result_buf; + mmfile_t result_buf; int ret = 0, merge_status; int two_way; diff --git a/notes-merge.c b/notes-merge.c index 118cad2518..d361e70a48 100644 --- a/notes-merge.c +++ b/notes-merge.c @@ -355,7 +355,7 @@ static void write_note_to_worktree(const struct object_id *obj, static int ll_merge_in_worktree(struct notes_merge_options *o, struct notes_merge_pair *p) { - mmbuffer_t result_buf; + mmfile_t result_buf; mmfile_t base, local, remote; enum ll_merge_result status; diff --git a/rerere.c b/rerere.c index 45bbe6ab2c..9b1aa932ab 100644 --- a/rerere.c +++ b/rerere.c @@ -600,7 +600,7 @@ int rerere_remaining(struct repository *r, struct string_list *merge_rr) */ static int try_merge(struct index_state *istate, const struct rerere_id *id, const char *path, - mmfile_t *cur, mmbuffer_t *result) + mmfile_t *cur, mmfile_t *result) { enum ll_merge_result ret; mmfile_t base = {NULL, 0}, other = {NULL, 0}; @@ -641,7 +641,7 @@ static int merge(struct index_state *istate, const struct rerere_id *id, const c int ret; struct strbuf buf = STRBUF_INIT; mmfile_t cur = {NULL, 0}; - mmbuffer_t result = {NULL, 0}; + mmfile_t result = {NULL, 0}; /* * Normalize the conflicts in path and write it out to @@ -969,7 +969,7 @@ static int handle_cache(struct index_state *istate, const char *path, unsigned char *hash, const char *output) { mmfile_t mmfile[3] = {{NULL}}; - mmbuffer_t result = {NULL, 0}; + mmfile_t result = {NULL, 0}; const struct cache_entry *ce; int pos, len, i, has_conflicts; struct rerere_io_mem io; @@ -1061,8 +1061,8 @@ static int rerere_forget_one_path(struct index_state *istate, for (id->variant = 0; id->variant < id->collection->status_nr; id->variant++) { - mmfile_t cur = { NULL, 0 }; - mmbuffer_t result = {NULL, 0}; + mmfile_t cur; + mmfile_t result = {NULL, 0}; int cleanly_resolved; if (!has_rerere_resolution(id)) @@ -1070,7 +1070,6 @@ static int rerere_forget_one_path(struct index_state *istate, handle_cache(istate, path, hash, rerere_path(&buf, id, "thisimage")); if (read_mmfile(&cur, rerere_path(&buf, id, "thisimage"))) { - free(cur.ptr); error(_("failed to update conflicted state in '%s'"), path); goto fail_exit; } diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh index 7bb601e117..5be3f056f5 100755 --- a/t/t4200-rerere.sh +++ b/t/t4200-rerere.sh @@ -734,4 +734,55 @@ test_expect_success 'rerere does not crash with unmatched conflict marker' ' test_must_fail git rebase --continue ' +test_expect_success 'rerere preserves conflicts when driver output is unreadable' ' + test_create_repo unreadable-output && + ( + cd unreadable-output && + git config rerere.enabled true && + git config rerere.autoupdate true && + write_script merge-driver <<-\EOF && + git merge-file "$@" + status=$? + if test -f fail-read + then + rm "$1" || exit 1 + fi + exit "$status" + EOF + git config merge.unreadable.driver "./merge-driver %A %O %B" && + echo "file merge=unreadable" >.gitattributes && + test_commit base file base && + git checkout -b one && + test_commit --no-tag one file one && + git checkout -b two base && + test_commit --no-tag two file two && + + # Teach rerere a resolution while the driver works normally. + test_must_fail git merge one && + echo resolved >file && + git rerere && + git merge --abort && + + # Recreate the conflict without replaying the resolution yet. + test_must_fail git -c rerere.enabled=false merge one && + + # We will expect the same conflicted content after rerere fails + # below. + cp file expect && + git ls-files -u >expect-index && + test_file_not_empty expect-index && + + # Now we try rerere again, but the merge driver will cause the + # read to fail. + >fail-read && + git rerere 2>err && + test_grep "Could not stat" err && + + # And we expect the conflicted state. + test_cmp expect file && + git ls-files -u >actual-index && + test_cmp expect-index actual-index + ) +' + test_done diff --git a/xdiff-interface.c b/xdiff-interface.c index db6938689f..b3e9f1952b 100644 --- a/xdiff-interface.c +++ b/xdiff-interface.c @@ -53,7 +53,7 @@ static int consume_one(void *priv_, char *s, unsigned long size) return 0; } -static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf) +static int xdiff_outf(void *priv_, mmfile_t *mb, int nbuf) { struct xdiff_emit_state *priv = priv_; int i; @@ -166,8 +166,9 @@ int read_mmfile(mmfile_t *ptr, const char *filename) if (!(f = fopen(filename, "rb"))) return error_errno("Could not open %s", filename); sz = xsize_t(st.st_size); - ptr->ptr = xmalloc(sz ? sz : 1); + ptr->ptr = xmallocz(sz); if (sz && fread(ptr->ptr, sz, 1, f) != 1) { + FREE_AND_NULL(ptr->ptr); fclose(f); return error("Could not read %s", filename); } diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h index dc370712e9..8fa513fc4e 100644 --- a/xdiff/xdiff.h +++ b/xdiff/xdiff.h @@ -70,14 +70,9 @@ extern "C" { typedef struct s_mmfile { char *ptr; - long size; + size_t size; } mmfile_t; -typedef struct s_mmbuffer { - char *ptr; - long size; -} mmbuffer_t; - typedef struct s_xpparam { unsigned long flags; @@ -96,7 +91,7 @@ typedef struct s_xdemitcb { long old_begin, long old_nr, long new_begin, long new_nr, const char *func, long funclen); - int (*out_line)(void *, mmbuffer_t *, int); + int (*out_line)(void *, mmfile_t *, int); } xdemitcb_t; typedef long (*find_func_t)(const char *line, long line_len, char *buffer, long buffer_size, void *priv); @@ -144,7 +139,7 @@ typedef struct s_xmparam { #define DEFAULT_CONFLICT_MARKER_SIZE 7 int xdl_merge(mmfile_t *orig, mmfile_t *mf1, mmfile_t *mf2, - xmparam_t const *xmp, mmbuffer_t *result); + xmparam_t const *xmp, mmfile_t *result); #ifdef __cplusplus } diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c index 659ad4ec97..7b37968d25 100644 --- a/xdiff/xmerge.c +++ b/xdiff/xmerge.c @@ -504,7 +504,7 @@ static int xdl_simplify_non_conflicts(xdfenv_t *xe1, xdmerge_t *m, */ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, xdfenv_t *xe2, xdchange_t *xscr2, - xmparam_t const *xmp, mmbuffer_t *result) + xmparam_t const *xmp, mmfile_t *result) { xdmerge_t *changes, *c; xpparam_t const *xpp = &xmp->xpp; @@ -682,7 +682,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, } int xdl_merge(mmfile_t *orig, mmfile_t *mf1, mmfile_t *mf2, - xmparam_t const *xmp, mmbuffer_t *result) + xmparam_t const *xmp, mmfile_t *result) { xdchange_t *xscr1 = NULL, *xscr2 = NULL; xdfenv_t xe1, xe2; diff --git a/xdiff/xutils.c b/xdiff/xutils.c index 9a999acdc0..4215f646c5 100644 --- a/xdiff/xutils.c +++ b/xdiff/xutils.c @@ -39,7 +39,7 @@ uint64_t xdl_bogosqrt(uint64_t n) { int xdl_emit_diffrec(char const *rec, long size, char const *pre, long psize, xdemitcb_t *ecb) { int i = 2; - mmbuffer_t mb[3]; + mmfile_t mb[3]; mb[0].ptr = (char *) pre; mb[0].size = psize; @@ -392,7 +392,7 @@ static int xdl_format_hunk_hdr(long s1, long c1, long s2, long c2, const char *func, long funclen, xdemitcb_t *ecb) { int nb = 0; - mmbuffer_t mb; + mmfile_t mb; char buf[128]; memcpy(buf, "@@ -", 4);