Add a write_entry() helper function to handle the unlinking and writing of the dir-diff submodule and symlink stand-in files. Use write_entry() inside of the hashmap loops to eliminate duplicate code and to safeguard the submodules hashmap loop against the symlink-chasing behavior that 5bafb3576a (difftool: fix symlink-file writing in dir-diff mode, 2021-09-22) addressed. The submodules loop should not strictly require the unlink() call that this is introducing to them, but it does not necessarily hurt them either beyond the cost of the extra unlink(). Signed-off-by: David Aguilar <davvid@xxxxxxxxx> --- This is cleanup refactoring that Junio suggested when 5bafb3576a (difftool: fix symlink-file writing in dir-diff mode, 2021-09-22) touched this area of the code. builtin/difftool.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/builtin/difftool.c b/builtin/difftool.c index e419bd3cd1..bbb8b399c2 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -320,6 +320,17 @@ static int checkout_path(unsigned mode, struct object_id *oid, return ret; } +static void write_entry(const char *path, const char *content, + struct strbuf *buf, size_t len) +{ + if (!*content) + return; + add_path(buf, len, path); + ensure_leading_directories(buf->buf); + unlink(buf->buf); + write_file(buf->buf, "%s", content); +} + static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, struct child_process *child) { @@ -533,16 +544,8 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, */ hashmap_for_each_entry(&submodules, &iter, entry, entry /* member name */) { - if (*entry->left) { - add_path(&ldir, ldir_len, entry->path); - ensure_leading_directories(ldir.buf); - write_file(ldir.buf, "%s", entry->left); - } - if (*entry->right) { - add_path(&rdir, rdir_len, entry->path); - ensure_leading_directories(rdir.buf); - write_file(rdir.buf, "%s", entry->right); - } + write_entry(entry->path, entry->left, &ldir, ldir_len); + write_entry(entry->path, entry->right, &rdir, rdir_len); } /* @@ -552,18 +555,9 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, */ hashmap_for_each_entry(&symlinks2, &iter, entry, entry /* member name */) { - if (*entry->left) { - add_path(&ldir, ldir_len, entry->path); - ensure_leading_directories(ldir.buf); - unlink(ldir.buf); - write_file(ldir.buf, "%s", entry->left); - } - if (*entry->right) { - add_path(&rdir, rdir_len, entry->path); - ensure_leading_directories(rdir.buf); - unlink(rdir.buf); - write_file(rdir.buf, "%s", entry->right); - } + + write_entry(entry->path, entry->left, &ldir, ldir_len); + write_entry(entry->path, entry->right, &rdir, rdir_len); } strbuf_release(&buf); -- 2.33.0.887.g8db6ae3373