[PATCH 1/4] Move remove_path() into libgit.a

Johannes Schindelin· Jan 24, 2007, 01:13 UTC · lore

Rename the function remove_path() to remove_path_recursively(), and move it to path.c.

This also fixes a bug in merge-recursive, where a now-empty directory was not removed.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 cache.h           |    1 +
 merge-recursive.c |   24 +-----------------------
 path.c            |   25 +++++++++++++++++++++++++
 3 files changed, 27 insertions(+), 23 deletions(-)

Show changes to 3 files +27 −23

cache.h, merge-recursive.c, path.c

diff --git a/cache.h b/cache.h
index a435781..64ea1dc 100644
--- a/cache.h
+++ b/cache.h
@@ -240,6 +240,7 @@ static inline void hashclr(unsigned char *hash)
 }
 
 int git_mkstemp(char *path, size_t n, const char *template);
+int remove_path_recursively(const char *name);
 
 enum sharedrepo {
 	PERM_UMASK = 0,
diff --git a/merge-recursive.c b/merge-recursive.c
index fa320eb..091177d 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -463,28 +463,6 @@ static int update_stages(const char *path, struct diff_filespec *o,
 	return 0;
 }
 
-static int remove_path(const char *name)
-{
-	int ret, len;
-	char *slash, *dirs;
-
-	ret = unlink(name);
-	if (ret)
-		return ret;
-	len = strlen(name);
-	dirs = xmalloc(len+1);
-	memcpy(dirs, name, len);
-	dirs[len] = '\0';
-	while ((slash = strrchr(name, '/'))) {
-		*slash = '\0';
-		len = slash - name;
-		if (rmdir(name) != 0)
-			break;
-	}
-	free(dirs);
-	return ret;
-}
-
 static int remove_file(int clean, const char *path, int no_wd)
 {
 	int update_cache = index_only || clean;
@@ -498,7 +476,7 @@ static int remove_file(int clean, const char *path, int no_wd)
 		unlink(path);
 		if (errno != ENOENT || errno != EISDIR)
 			return -1;
-		remove_path(path);
+		remove_path_recursively(path);
 	}
 	return 0;
 }
diff --git a/path.c b/path.c
index c5d25a4..b1f64e6 100644
--- a/path.c
+++ b/path.c
@@ -292,3 +292,28 @@ int adjust_shared_perm(const char *path)
 		return -2;
 	return 0;
 }
+
+/* this also removes the directories which became empty */
+int remove_path_recursively(const char *name)
+{
+	int ret, len;
+	char *slash, *dirs;
+
+	ret = unlink(name);
+	if (ret == ENOENT || ret == EISDIR)
+		ret = 0;
+	if (ret)
+		return ret;
+	len = strlen(name);
+	dirs = xmalloc(len+1);
+	memcpy(dirs, name, len);
+	dirs[len] = '\0';
+	while ((slash = strrchr(name, '/'))) {
+		*slash = '\0';
+		len = slash - name;
+		if (rmdir(name) != 0)
+			break;
+	}
+	free(dirs);
+	return ret;
+}

-- 
1.5.0.rc2.gc5868-dirty

Junio C Hamano· Jan 25, 2007, 00:07 UTC · re: Johannes Schindelin · lore

Re: [PATCH 1/4] Move remove_path() into libgit.a

We seem to have variants of the seme theme in many places.

 * builtin-apply.c::remove_file()
 * builtin-rm.c::remove_file() -- this one leaks the strdup result
 * refs.c::remove_empty_dir_recursive()
 * unpack-trees.c::unlink_entry() 
 * mrege-recursive.c::remove_path()

It would be nicer to consolidate their "remove the now potentially empty directories recursively" part in a single function and have them call it.

Some of them try to be more efficient by not allocating an extra string (because they know the string supplied by the caller is a fair game), so the clean-up might be slightly involved, but I think it is worth it.

patch, 4 partsMove remove_path() into libgit.a