Skip to content

Commit 81db32d

Browse files
gitstergitster
committed
Merge branch 'jk/xdiff-size_t'
The internal diff and merge machinery has been updated to use standard size_t types rather than signed longs for buffer allocations and offsets, preventing subtle overflow issues on LLP64 architectures, and plugging uninitialized memory leaks along error paths. * jk/xdiff-size_t: merge-ll: report an error when reading external merge results fails merge-ll: handle external driver status before reading result merge-ll: use read_mmfile() to read external merge results xdiff: NUL-terminate buffers read by read_mmfile() xdiff: use size_t for buffer sizes xdiff: replace mmbuffer_t with mmfile_t xdiff: clean up read_mmfile() allocations on error
2 parents 1ee477a + 5b04563 commit 81db32d

18 files changed

Lines changed: 101 additions & 63 deletions

‎Documentation/technical/api-merge.adoc‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,10 @@ responsible for a few things.
2020
Data structures
2121
---------------
2222
23-
* `mmbuffer_t`, `mmfile_t`
23+
* `mmfile_t`
2424
25-
These store data usable for use by the xdiff backend, for writing and
26-
for reading, respectively. See `xdiff/xdiff.h` for the definitions
27-
and `diff.c` for examples.
25+
This stores a buffer and its size for input to or output from the xdiff
26+
backend. See `xdiff/xdiff.h` for the definition and `diff.c` for examples.
2827
2928
* `struct ll_merge_options`
3029

‎apply.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3647,7 +3647,7 @@ static int three_way_merge(struct apply_state *state,
36473647
{
36483648
mmfile_t base_file, our_file, their_file;
36493649
struct ll_merge_options merge_opts = LL_MERGE_OPTIONS_INIT;
3650-
mmbuffer_t result = { NULL };
3650+
mmfile_t result = { NULL };
36513651
enum ll_merge_result status;
36523652

36533653
/* resolve trivial cases first */

‎builtin/checkout.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ static int checkout_merged(int pos, const struct checkout *state,
320320
enum ll_merge_result merge_status;
321321
int status;
322322
struct object_id oid;
323-
mmbuffer_t result_buf;
323+
mmfile_t result_buf;
324324
struct object_id threeway[3];
325325
unsigned mode = 0;
326326
struct ll_merge_options ll_opts = LL_MERGE_OPTIONS_INIT;

‎builtin/merge-file.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ int cmd_merge_file(int argc,
6464
{
6565
const char *names[3] = { 0 };
6666
mmfile_t mmfs[3] = { 0 };
67-
mmbuffer_t result = { 0 };
67+
mmfile_t result = { 0 };
6868
xmparam_t xmp = { 0 };
6969
int ret = 0, i = 0, to_stdout = 0, object_id = 0;
7070
int quiet = 0;

‎builtin/merge-tree.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ static void *origin(struct merge_list *entry, size_t *size)
109109
return NULL;
110110
}
111111

112-
static int show_outf(void *priv UNUSED, mmbuffer_t *mb, int nbuf)
112+
static int show_outf(void *priv UNUSED, mmfile_t *mb, int nbuf)
113113
{
114114
int i;
115115
for (i = 0; i < nbuf; i++)

‎builtin/rerere.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ static const char * const rerere_usage[] = {
1616
NULL,
1717
};
1818

19-
static int outf(void *dummy UNUSED, mmbuffer_t *ptr, int nbuf)
19+
static int outf(void *dummy UNUSED, mmfile_t *ptr, int nbuf)
2020
{
2121
int i;
2222
for (i = 0; i < nbuf; i++)
@@ -34,8 +34,12 @@ static int diff_two(const char *file1, const char *label1,
3434
mmfile_t minus, plus;
3535
int ret;
3636

37-
if (read_mmfile(&minus, file1) || read_mmfile(&plus, file2))
37+
if (read_mmfile(&minus, file1))
3838
return -1;
39+
if (read_mmfile(&plus, file2)) {
40+
free(minus.ptr);
41+
return -1;
42+
}
3943

4044
printf("--- a/%s\n+++ b/%s\n", label1, label2);
4145
fflush(stdout);

‎diff.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3646,7 +3646,7 @@ static void emit_binary_diff_body(struct diff_options *o,
36463646
data = delta;
36473647
data_size = delta_size;
36483648
} else {
3649-
char *s = xstrfmt("%lu", two->size);
3649+
char *s = xstrfmt("%"PRIuMAX, (uintmax_t)two->size);
36503650
emit_diff_symbol(o, DIFF_SYMBOL_BINARY_DIFF_HEADER_LITERAL,
36513651
s, strlen(s), 0);
36523652
free(s);

‎merge-blobs.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ static void *three_way_filemerge(struct index_state *istate,
3838
size_t *size)
3939
{
4040
enum ll_merge_result merge_status;
41-
mmbuffer_t res;
41+
mmfile_t res;
4242

4343
/*
4444
* This function is only used by cmd_merge_tree, which

‎merge-ll.c‎

Lines changed: 15 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
struct ll_merge_driver;
2323

2424
typedef enum ll_merge_result (*ll_merge_fn)(const struct ll_merge_driver *,
25-
mmbuffer_t *result,
25+
mmfile_t *result,
2626
const char *path,
2727
mmfile_t *orig, const char *orig_name,
2828
mmfile_t *src1, const char *name1,
@@ -57,7 +57,7 @@ void reset_merge_attributes(void)
5757
* Built-in low-levels
5858
*/
5959
static enum ll_merge_result ll_binary_merge(const struct ll_merge_driver *drv UNUSED,
60-
mmbuffer_t *result,
60+
mmfile_t *result,
6161
const char *path UNUSED,
6262
mmfile_t *orig, const char *orig_name UNUSED,
6363
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
102102
}
103103

104104
static enum ll_merge_result ll_xdl_merge(const struct ll_merge_driver *drv_unused,
105-
mmbuffer_t *result,
105+
mmfile_t *result,
106106
const char *path,
107107
mmfile_t *orig, const char *orig_name,
108108
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
148148
}
149149

150150
static enum ll_merge_result ll_union_merge(const struct ll_merge_driver *drv_unused,
151-
mmbuffer_t *result,
151+
mmfile_t *result,
152152
const char *path,
153153
mmfile_t *orig, const char *orig_name,
154154
mmfile_t *src1, const char *name1,
@@ -203,7 +203,7 @@ static const char *temp_path_basename(struct tempfile *t)
203203
* User defined low-level merge driver support.
204204
*/
205205
static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
206-
mmbuffer_t *result,
206+
mmfile_t *result,
207207
const char *path,
208208
mmfile_t *orig, const char *orig_name,
209209
mmfile_t *src1, const char *name1,
@@ -215,8 +215,7 @@ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
215215
struct strbuf cmd = STRBUF_INIT;
216216
const char *format = fn->cmdline;
217217
struct child_process child = CHILD_PROCESS_INIT;
218-
int status, fd;
219-
struct stat st;
218+
int status;
220219
enum ll_merge_result ret;
221220
assert(opts);
222221

@@ -255,31 +254,21 @@ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
255254
child.use_shell = 1;
256255
strvec_push(&child.args, cmd.buf);
257256
status = run_command(&child);
258-
fd = open(get_tempfile_path(tmp_a), O_RDONLY);
259-
if (fd < 0)
260-
goto bad;
261-
if (fstat(fd, &st))
262-
goto close_bad;
263-
result->size = st.st_size;
264-
result->ptr = xmallocz(result->size);
265-
if (read_in_full(fd, result->ptr, result->size) != result->size) {
266-
FREE_AND_NULL(result->ptr);
267-
result->size = 0;
268-
}
269-
close_bad:
270-
close(fd);
271-
bad:
272-
delete_tempfile(&tmp_o);
273-
delete_tempfile(&tmp_a);
274-
delete_tempfile(&tmp_b);
275-
strbuf_release(&cmd);
276257
if (!status)
277258
ret = LL_MERGE_OK;
278259
else if (status <= 128)
279260
ret = LL_MERGE_CONFLICT;
280261
else
281262
/* died due to a signal: WTERMSIG(status) + 128 */
282263
ret = LL_MERGE_ERROR;
264+
265+
if (read_mmfile(result, temp_path_basename(tmp_a)) < 0)
266+
ret = LL_MERGE_ERROR;
267+
268+
delete_tempfile(&tmp_o);
269+
delete_tempfile(&tmp_a);
270+
delete_tempfile(&tmp_b);
271+
strbuf_release(&cmd);
283272
return ret;
284273
}
285274

@@ -418,7 +407,7 @@ static void normalize_file(mmfile_t *mm, const char *path, struct index_state *i
418407
}
419408
}
420409

421-
enum ll_merge_result ll_merge(mmbuffer_t *result_buf,
410+
enum ll_merge_result ll_merge(mmfile_t *result_buf,
422411
const char *path,
423412
mmfile_t *ancestor, const char *ancestor_label,
424413
mmfile_t *ours, const char *our_label,

‎merge-ll.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* If you have no special requests, skip this and pass `NULL`
1717
* as the `opts` parameter to use the default options.
1818
*
19-
* - Allocate an mmbuffer_t variable for the result.
19+
* - Allocate an mmfile_t variable for the result.
2020
*
2121
* - Allocate and fill variables with the file's original content
2222
* and two modified versions (using `read_mmfile`, for example).
@@ -100,7 +100,7 @@ enum ll_merge_result {
100100
* `.gitattributes` or `.git/info/attributes` into account.
101101
* Returns 0 for a clean merge.
102102
*/
103-
enum ll_merge_result ll_merge(mmbuffer_t *result_buf,
103+
enum ll_merge_result ll_merge(mmfile_t *result_buf,
104104
const char *path,
105105
mmfile_t *ancestor, const char *ancestor_label,
106106
mmfile_t *ours, const char *our_label,

0 commit comments

Comments
 (0)