Skip to content

Commit 4ac9f15

Browse files
davvidgitster
davvid
authored and
gitster
committed
difftool: create a tmpdir path without repeated slashes
The paths generated by difftool are passed to user-facing diff tools. Using paths with repeated slashes in them is a cosmetic blemish that is exposed to users and can be avoided. Use a strbuf to create the buffer used for the dir-diff tmpdir. Strip trailing slashes from the value read from TMPDIR to avoid repeated slashes in the generated paths. Adjust the error handling to avoid leaking strbufs and to avoid returning -1 to cmd_main(). Signed-off-by: David Aguilar <davvid@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 77bd616 commit 4ac9f15

File tree

2 files changed

+31
-26
lines changed

2 files changed

+31
-26
lines changed

‎builtin/difftool.c‎

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -252,16 +252,6 @@ static void changed_files(struct hashmap *result, const char *index_path,
252252
strbuf_release(&buf);
253253
}
254254

255-
static NORETURN void exit_cleanup(const char *tmpdir, int exit_code)
256-
{
257-
struct strbuf buf = STRBUF_INIT;
258-
strbuf_addstr(&buf, tmpdir);
259-
remove_dir_recursively(&buf, 0);
260-
if (exit_code)
261-
warning(_("failed: %d"), exit_code);
262-
exit(exit_code);
263-
}
264-
265255
static int ensure_leading_directories(char *path)
266256
{
267257
switch (safe_create_leading_directories(path)) {
@@ -333,16 +323,16 @@ static int checkout_path(unsigned mode, struct object_id *oid,
333323
static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
334324
struct child_process *child)
335325
{
336-
char tmpdir[PATH_MAX];
337326
struct strbuf info = STRBUF_INIT, lpath = STRBUF_INIT;
338327
struct strbuf rpath = STRBUF_INIT, buf = STRBUF_INIT;
339328
struct strbuf ldir = STRBUF_INIT, rdir = STRBUF_INIT;
340329
struct strbuf wtdir = STRBUF_INIT;
341-
char *lbase_dir, *rbase_dir;
330+
struct strbuf tmpdir = STRBUF_INIT;
331+
char *lbase_dir = NULL, *rbase_dir = NULL;
342332
size_t ldir_len, rdir_len, wtdir_len;
343333
const char *workdir, *tmp;
344334
int ret = 0, i;
345-
FILE *fp;
335+
FILE *fp = NULL;
346336
struct hashmap working_tree_dups = HASHMAP_INIT(working_tree_entry_cmp,
347337
NULL);
348338
struct hashmap submodules = HASHMAP_INIT(pair_cmp, NULL);
@@ -351,7 +341,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
351341
struct pair_entry *entry;
352342
struct index_state wtindex;
353343
struct checkout lstate, rstate;
354-
int rc, flags = RUN_GIT_CMD, err = 0;
344+
int flags = RUN_GIT_CMD, err = 0;
355345
const char *helper_argv[] = { "difftool--helper", NULL, NULL, NULL };
356346
struct hashmap wt_modified, tmp_modified;
357347
int indices_loaded = 0;
@@ -360,11 +350,15 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
360350

361351
/* Setup temp directories */
362352
tmp = getenv("TMPDIR");
363-
xsnprintf(tmpdir, sizeof(tmpdir), "%s/git-difftool.XXXXXX", tmp ? tmp : "/tmp");
364-
if (!mkdtemp(tmpdir))
365-
return error("could not create '%s'", tmpdir);
366-
strbuf_addf(&ldir, "%s/left/", tmpdir);
367-
strbuf_addf(&rdir, "%s/right/", tmpdir);
353+
strbuf_add_absolute_path(&tmpdir, tmp ? tmp : "/tmp");
354+
strbuf_trim_trailing_dir_sep(&tmpdir);
355+
strbuf_addstr(&tmpdir, "/git-difftool.XXXXXX");
356+
if (!mkdtemp(tmpdir.buf)) {
357+
ret = error("could not create '%s'", tmpdir.buf);
358+
goto finish;
359+
}
360+
strbuf_addf(&ldir, "%s/left/", tmpdir.buf);
361+
strbuf_addf(&rdir, "%s/right/", tmpdir.buf);
368362
strbuf_addstr(&wtdir, workdir);
369363
if (!wtdir.len || !is_dir_sep(wtdir.buf[wtdir.len - 1]))
370364
strbuf_addch(&wtdir, '/');
@@ -580,7 +574,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
580574
flags = 0;
581575
} else
582576
setenv("GIT_DIFFTOOL_DIRDIFF", "true", 1);
583-
rc = run_command_v_opt(helper_argv, flags);
577+
ret = run_command_v_opt(helper_argv, flags);
584578

585579
/* TODO: audit for interaction with sparse-index. */
586580
ensure_full_index(&wtindex);
@@ -614,7 +608,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
614608
if (!indices_loaded) {
615609
struct lock_file lock = LOCK_INIT;
616610
strbuf_reset(&buf);
617-
strbuf_addf(&buf, "%s/wtindex", tmpdir);
611+
strbuf_addf(&buf, "%s/wtindex", tmpdir.buf);
618612
if (hold_lock_file_for_update(&lock, buf.buf, 0) < 0 ||
619613
write_locked_index(&wtindex, &lock, COMMIT_LOCK)) {
620614
ret = error("could not write %s", buf.buf);
@@ -644,11 +638,14 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
644638
}
645639

646640
if (err) {
647-
warning(_("temporary files exist in '%s'."), tmpdir);
641+
warning(_("temporary files exist in '%s'."), tmpdir.buf);
648642
warning(_("you may want to cleanup or recover these."));
649-
exit(1);
650-
} else
651-
exit_cleanup(tmpdir, rc);
643+
ret = 1;
644+
} else {
645+
remove_dir_recursively(&tmpdir, 0);
646+
if (ret)
647+
warning(_("failed: %d"), ret);
648+
}
652649

653650
finish:
654651
if (fp)
@@ -660,8 +657,9 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
660657
strbuf_release(&rdir);
661658
strbuf_release(&wtdir);
662659
strbuf_release(&buf);
660+
strbuf_release(&tmpdir);
663661

664-
return ret;
662+
return (ret < 0) ? 1 : ret;
665663
}
666664

667665
static int run_file_diff(int prompt, const char *prefix,

‎t/t7800-difftool.sh‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,13 @@ run_dir_diff_test 'difftool --dir-diff' '
453453
grep "^file$" output
454454
'
455455

456+
run_dir_diff_test 'difftool --dir-diff avoids repeated slashes in TMPDIR' '
457+
TMPDIR="${TMPDIR:-/tmp}////" \
458+
git difftool --dir-diff $symlinks --extcmd echo branch >output &&
459+
grep -v // output >actual &&
460+
test_line_count = 1 actual
461+
'
462+
456463
run_dir_diff_test 'difftool --dir-diff ignores --prompt' '
457464
git difftool --dir-diff $symlinks --prompt --extcmd ls branch >output &&
458465
grep "^sub$" output &&

0 commit comments

Comments
 (0)