Subject: [PATCH v6 4/5] difftool: refactor dir-diff to write files using a helper function

From: David Aguilar <[email-masked]>
Date: Thu, 30 Sep 2021 10:01:45 -0700

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 <[email-masked]>
---
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