[PATCH] rerere: remove duplicated functions

[PATCH] rerere: remove duplicated functions

[SZEDER Gábor]

Both rerere.c and builtin-rerere.c define the static functions
rr_path() and has_resolution() the exact same way.  To eliminate this
code duplication this patch turns the functions in rerere.c
non-static, and makes builtin-rerere.c use them.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 builtin-rerere.c |   11 -----------
 rerere.c         |    4 ++--
 rerere.h         |    2 ++
 3 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/builtin-rerere.c b/builtin-rerere.c
index bd8fc77..b175334 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -13,23 +13,12 @@ static const char git_rerere_usage[] =
 static int cutoff_noresolve = 15;
 static int cutoff_resolve = 60;
 
-static const char *rr_path(const char *name, const char *file)
-{
-	return git_path("rr-cache/%s/%s", name, file);
-}
-
 static time_t rerere_created_at(const char *name)
 {
 	struct stat st;
 	return stat(rr_path(name, "preimage"), &st) ? (time_t) 0 : st.st_mtime;
 }
 
-static int has_resolution(const char *name)
-{
-	struct stat st;
-	return !stat(rr_path(name, "postimage"), &st);
-}
-
 static void unlink_rr_item(const char *name)
 {
 	unlink(rr_path(name, "thisimage"));
diff --git a/rerere.c b/rerere.c
index 3518207..ac89b80 100644
--- a/rerere.c
+++ b/rerere.c
@@ -12,12 +12,12 @@ static int rerere_autoupdate;
 
 static char *merge_rr_path;
 
-static const char *rr_path(const char *name, const char *file)
+const char *rr_path(const char *name, const char *file)
 {
 	return git_path("rr-cache/%s/%s", name, file);
 }
 
-static int has_resolution(const char *name)
+int has_resolution(const char *name)
 {
 	struct stat st;
 	return !stat(rr_path(name, "postimage"), &st);
diff --git a/rerere.h b/rerere.h
index f9b0386..45b5087 100644
--- a/rerere.h
+++ b/rerere.h
@@ -5,5 +5,7 @@
 
 extern int setup_rerere(struct string_list *);
 extern int rerere(void);
+extern const char *rr_path(const char *name, const char *file);
+extern int has_resolution(const char *name);
 
 #endif
-- 
1.6.2.rc0.111.g246ed

Re: [PATCH] rerere: remove duplicated functions

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 744 bytes --]

Hi,

On Sat, 14 Feb 2009, SZEDER Gábor wrote:

> diff --git a/rerere.h b/rerere.h
> index f9b0386..45b5087 100644
> --- a/rerere.h
> +++ b/rerere.h
> @@ -5,5 +5,7 @@
>  
>  extern int setup_rerere(struct string_list *);
>  extern int rerere(void);
> +extern const char *rr_path(const char *name, const char *file);
> +extern int has_resolution(const char *name);

You might argue that rr_path() is a unique enough name (although I'd 
rather name it rerere_path() instead), but has_resolution() is not a good 
description of what the function does when it lives in the global name 
space.  has_rerere_resolution() (with "hex" instead of "name", to make it 
obvious what the parameter actually _is_) should be a much better name.

Thanks,
Dscho

[PATCH] rerere: remove duplicated functions

[SZEDER Gábor]

Both rerere.c and builtin-rerere.c define the static functions
rr_path() and has_resolution() the exact same way.  To eliminate this
code duplication this patch turns the functions in rerere.c
non-static, and makes builtin-rerere.c use them.  Also, since this
puts these two functions into the global namespace, rename them to
rerere_path() and has_rerere_resolution(), respectively, and rename
their "name" parameter to "hex", because it better reflects what that
parameter actually is.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---

  On Sat, Feb 14, 2009 at 07:47:36PM +0100, Johannes Schindelin wrote:
  > > +extern const char *rr_path(const char *name, const char *file);
  > > +extern int has_resolution(const char *name);
  > 
  > You might argue that rr_path() is a unique enough name (although I'd
  > rather name it rerere_path() instead), but has_resolution() is not a
  > good description of what the function does when it lives in the global
  > name space.  has_rerere_resolution() (with "hex" instead of "name", to
  > make it obvious what the parameter actually _is_) should be a much
  > better name.

  You are right;  I was aiming for a minimal change and haven't noticed
  the naming issue.  Renamed both functions and their "name" parameter,
  as you suggested.


 builtin-rerere.c |   25 +++++++------------------
 rerere.c         |   22 +++++++++++-----------
 rerere.h         |    2 ++
 3 files changed, 20 insertions(+), 29 deletions(-)

diff --git a/builtin-rerere.c b/builtin-rerere.c
index bd8fc77..020af73 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -13,28 +13,17 @@ static const char git_rerere_usage[] =
 static int cutoff_noresolve = 15;
 static int cutoff_resolve = 60;
 
-static const char *rr_path(const char *name, const char *file)
-{
-	return git_path("rr-cache/%s/%s", name, file);
-}
-
 static time_t rerere_created_at(const char *name)
 {
 	struct stat st;
-	return stat(rr_path(name, "preimage"), &st) ? (time_t) 0 : st.st_mtime;
-}
-
-static int has_resolution(const char *name)
-{
-	struct stat st;
-	return !stat(rr_path(name, "postimage"), &st);
+	return stat(rerere_path(name, "preimage"), &st) ? (time_t) 0 : st.st_mtime;
 }
 
 static void unlink_rr_item(const char *name)
 {
-	unlink(rr_path(name, "thisimage"));
-	unlink(rr_path(name, "preimage"));
-	unlink(rr_path(name, "postimage"));
+	unlink(rerere_path(name, "thisimage"));
+	unlink(rerere_path(name, "preimage"));
+	unlink(rerere_path(name, "postimage"));
 	rmdir(git_path("rr-cache/%s", name));
 }
 
@@ -65,7 +54,7 @@ static void garbage_collect(struct string_list *rr)
 		then = rerere_created_at(e->d_name);
 		if (!then)
 			continue;
-		cutoff = (has_resolution(e->d_name)
+		cutoff = (has_rerere_resolution(e->d_name)
 			  ? cutoff_resolve : cutoff_noresolve);
 		if (then < now - cutoff * 86400)
 			string_list_append(e->d_name, &to_remove);
@@ -124,7 +113,7 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
 	if (!strcmp(argv[1], "clear")) {
 		for (i = 0; i < merge_rr.nr; i++) {
 			const char *name = (const char *)merge_rr.items[i].util;
-			if (!has_resolution(name))
+			if (!has_rerere_resolution(name))
 				unlink_rr_item(name);
 		}
 		unlink(git_path("rr-cache/MERGE_RR"));
@@ -137,7 +126,7 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
 		for (i = 0; i < merge_rr.nr; i++) {
 			const char *path = merge_rr.items[i].string;
 			const char *name = (const char *)merge_rr.items[i].util;
-			diff_two(rr_path(name, "preimage"), path, path, path);
+			diff_two(rerere_path(name, "preimage"), path, path, path);
 		}
 	else
 		usage(git_rerere_usage);
diff --git a/rerere.c b/rerere.c
index 3518207..713c6e1 100644
--- a/rerere.c
+++ b/rerere.c
@@ -12,15 +12,15 @@ static int rerere_autoupdate;
 
 static char *merge_rr_path;
 
-static const char *rr_path(const char *name, const char *file)
+const char *rerere_path(const char *hex, const char *file)
 {
-	return git_path("rr-cache/%s/%s", name, file);
+	return git_path("rr-cache/%s/%s", hex, file);
 }
 
-static int has_resolution(const char *name)
+int has_rerere_resolution(const char *hex)
 {
 	struct stat st;
-	return !stat(rr_path(name, "postimage"), &st);
+	return !stat(rerere_path(hex, "postimage"), &st);
 }
 
 static void read_rr(struct string_list *rr)
@@ -208,12 +208,12 @@ static int merge(const char *name, const char *path)
 	mmbuffer_t result = {NULL, 0};
 	xpparam_t xpp = {XDF_NEED_MINIMAL};
 
-	if (handle_file(path, NULL, rr_path(name, "thisimage")) < 0)
+	if (handle_file(path, NULL, rerere_path(name, "thisimage")) < 0)
 		return 1;
 
-	if (read_mmfile(&cur, rr_path(name, "thisimage")) ||
-			read_mmfile(&base, rr_path(name, "preimage")) ||
-			read_mmfile(&other, rr_path(name, "postimage")))
+	if (read_mmfile(&cur, rerere_path(name, "thisimage")) ||
+			read_mmfile(&base, rerere_path(name, "preimage")) ||
+			read_mmfile(&other, rerere_path(name, "postimage")))
 		return 1;
 	ret = xdl_merge(&base, &cur, "", &other, "",
 			&xpp, XDL_MERGE_ZEALOUS, &result);
@@ -291,7 +291,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 			string_list_insert(path, rr)->util = hex;
 			if (mkdir(git_path("rr-cache/%s", hex), 0755))
 				continue;
-			handle_file(path, NULL, rr_path(hex, "preimage"));
+			handle_file(path, NULL, rerere_path(hex, "preimage"));
 			fprintf(stderr, "Recorded preimage for '%s'\n", path);
 		}
 	}
@@ -307,7 +307,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 		const char *path = rr->items[i].string;
 		const char *name = (const char *)rr->items[i].util;
 
-		if (has_resolution(name)) {
+		if (has_rerere_resolution(name)) {
 			if (!merge(name, path)) {
 				if (rerere_autoupdate)
 					string_list_insert(path, &update);
@@ -326,7 +326,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 			continue;
 
 		fprintf(stderr, "Recorded resolution for '%s'.\n", path);
-		copy_file(rr_path(name, "postimage"), path, 0666);
+		copy_file(rerere_path(name, "postimage"), path, 0666);
 	mark_resolved:
 		rr->items[i].util = NULL;
 	}
diff --git a/rerere.h b/rerere.h
index f9b0386..13313f3 100644
--- a/rerere.h
+++ b/rerere.h
@@ -5,5 +5,7 @@
 
 extern int setup_rerere(struct string_list *);
 extern int rerere(void);
+extern const char *rerere_path(const char *hex, const char *file);
+extern int has_rerere_resolution(const char *hex);
 
 #endif
-- 
1.6.2.rc0.111.g246ed

Re: [PATCH] rerere: remove duplicated functions

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 629 bytes --]

Hi,

On Sat, 14 Feb 2009, SZEDER Gábor wrote:

> Both rerere.c and builtin-rerere.c define the static functions
> rr_path() and has_resolution() the exact same way.  To eliminate this
> code duplication this patch turns the functions in rerere.c
> non-static, and makes builtin-rerere.c use them.  Also, since this
> puts these two functions into the global namespace, rename them to
> rerere_path() and has_rerere_resolution(), respectively, and rename
> their "name" parameter to "hex", because it better reflects what that
> parameter actually is.

Thanks.  As I started that code in C, I dare say "ACK" here :-)

Ciao,
Dscho

Re: [PATCH] rerere: remove duplicated functions

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Thanks.  As I started that code in C, I dare say "ACK" here :-)

Thanks, both.