Skip to content

Commit 7b612c9

Browse files
gitstergitster
committed
Merge branch 'js/difftool-builtin'
Code cleanup. * js/difftool-builtin: difftool: fix use-after-free difftool: avoid strcpy
2 parents c8a8951 + 882add1 commit 7b612c9

File tree

2 files changed

+39
-18
lines changed

2 files changed

+39
-18
lines changed

‎builtin/difftool.c‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,19 @@ static char *get_symlink(const struct object_id *oid, const char *path)
297297
return data;
298298
}
299299

300+
static int checkout_path(unsigned mode, struct object_id *oid,
301+
const char *path, const struct checkout *state)
302+
{
303+
struct cache_entry *ce;
304+
int ret;
305+
306+
ce = make_cache_entry(mode, oid->hash, path, 0, 0);
307+
ret = checkout_entry(ce, state, NULL);
308+
309+
free(ce);
310+
return ret;
311+
}
312+
300313
static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
301314
int argc, const char **argv)
302315
{
@@ -305,8 +318,8 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
305318
struct strbuf rpath = STRBUF_INIT, buf = STRBUF_INIT;
306319
struct strbuf ldir = STRBUF_INIT, rdir = STRBUF_INIT;
307320
struct strbuf wtdir = STRBUF_INIT;
321+
char *lbase_dir, *rbase_dir;
308322
size_t ldir_len, rdir_len, wtdir_len;
309-
struct cache_entry *ce = xcalloc(1, sizeof(ce) + PATH_MAX + 1);
310323
const char *workdir, *tmp;
311324
int ret = 0, i;
312325
FILE *fp;
@@ -339,11 +352,11 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
339352
memset(&wtindex, 0, sizeof(wtindex));
340353

341354
memset(&lstate, 0, sizeof(lstate));
342-
lstate.base_dir = ldir.buf;
355+
lstate.base_dir = lbase_dir = xstrdup(ldir.buf);
343356
lstate.base_dir_len = ldir.len;
344357
lstate.force = 1;
345358
memset(&rstate, 0, sizeof(rstate));
346-
rstate.base_dir = rdir.buf;
359+
rstate.base_dir = rbase_dir = xstrdup(rdir.buf);
347360
rstate.base_dir_len = rdir.len;
348361
rstate.force = 1;
349362

@@ -377,7 +390,6 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
377390
struct object_id loid, roid;
378391
char status;
379392
const char *src_path, *dst_path;
380-
size_t src_path_len, dst_path_len;
381393

382394
if (starts_with(info.buf, "::"))
383395
die(N_("combined diff formats('-c' and '--cc') are "
@@ -390,17 +402,14 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
390402
if (strbuf_getline_nul(&lpath, fp))
391403
break;
392404
src_path = lpath.buf;
393-
src_path_len = lpath.len;
394405

395406
i++;
396407
if (status != 'C' && status != 'R') {
397408
dst_path = src_path;
398-
dst_path_len = src_path_len;
399409
} else {
400410
if (strbuf_getline_nul(&rpath, fp))
401411
break;
402412
dst_path = rpath.buf;
403-
dst_path_len = rpath.len;
404413
}
405414

406415
if (S_ISGITLINK(lmode) || S_ISGITLINK(rmode)) {
@@ -430,11 +439,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
430439
}
431440

432441
if (lmode && status != 'C') {
433-
ce->ce_mode = lmode;
434-
oidcpy(&ce->oid, &loid);
435-
strcpy(ce->name, src_path);
436-
ce->ce_namelen = src_path_len;
437-
if (checkout_entry(ce, &lstate, NULL))
442+
if (checkout_path(lmode, &loid, src_path, &lstate))
438443
return error("could not write '%s'", src_path);
439444
}
440445

@@ -451,11 +456,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
451456
hashmap_add(&working_tree_dups, entry);
452457

453458
if (!use_wt_file(workdir, dst_path, &roid)) {
454-
ce->ce_mode = rmode;
455-
oidcpy(&ce->oid, &roid);
456-
strcpy(ce->name, dst_path);
457-
ce->ce_namelen = dst_path_len;
458-
if (checkout_entry(ce, &rstate, NULL))
459+
if (checkout_path(rmode, &roid, dst_path, &rstate))
459460
return error("could not write '%s'",
460461
dst_path);
461462
} else if (!is_null_oid(&roid)) {
@@ -625,7 +626,8 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
625626
exit_cleanup(tmpdir, rc);
626627

627628
finish:
628-
free(ce);
629+
free(lbase_dir);
630+
free(rbase_dir);
629631
strbuf_release(&ldir);
630632
strbuf_release(&rdir);
631633
strbuf_release(&wtdir);

‎t/t7800-difftool.sh‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,25 @@ test_expect_success 'setup change in subdirectory' '
393393
git commit -m "modified both"
394394
'
395395

396+
test_expect_success 'difftool -d with growing paths' '
397+
a=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa &&
398+
git init growing &&
399+
(
400+
cd growing &&
401+
echo "test -f \"\$2/b\"" | write_script .git/test-for-b.sh &&
402+
one=$(printf 1 | git hash-object -w --stdin) &&
403+
two=$(printf 2 | git hash-object -w --stdin) &&
404+
git update-index --add \
405+
--cacheinfo 100644,$one,$a --cacheinfo 100644,$two,b &&
406+
tree1=$(git write-tree) &&
407+
git update-index --add \
408+
--cacheinfo 100644,$two,$a --cacheinfo 100644,$one,b &&
409+
tree2=$(git write-tree) &&
410+
git checkout -- $a &&
411+
git difftool -d --extcmd .git/test-for-b.sh $tree1 $tree2
412+
)
413+
'
414+
396415
run_dir_diff_test () {
397416
test_expect_success "$1 --no-symlinks" "
398417
symlinks=--no-symlinks &&

0 commit comments

Comments
 (0)