{"thread":{"id":"17789","subject":"[PATCH] rerere: remove duplicated functions","startedAt":"2009-02-14T16:18:04Z","lastAt":"2009-02-15T19:33:45Z","messageCount":5,"participants":["SZEDER Gábor","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"104643","messageId":"1234628284-4246-1-git-send-email-szeder@ira.uka.de","threadId":"17789","inReplyTo":null,"subject":"[PATCH] rerere: remove duplicated functions","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2009-02-14T16:18:04Z","receivedAt":"2009-02-14T16:18:04Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Both rerere.c and builtin-rerere.c define the static functions\nrr_path() and has_resolution() the exact same way.  To eliminate this\ncode duplication this patch turns the functions in rerere.c\nnon-static, and makes builtin-rerere.c use them.\n\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\n builtin-rerere.c |   11 -----------\n rerere.c         |    4 ++--\n rerere.h         |    2 ++\n 3 files changed, 4 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin-rerere.c b/builtin-rerere.c\nindex bd8fc77..b175334 100644\n--- a/builtin-rerere.c\n+++ b/builtin-rerere.c\n@@ -13,23 +13,12 @@ static const char git_rerere_usage[] =\n static int cutoff_noresolve = 15;\n static int cutoff_resolve = 60;\n \n-static const char *rr_path(const char *name, const char *file)\n-{\n-\treturn git_path(\"rr-cache/%s/%s\", name, file);\n-}\n-\n static time_t rerere_created_at(const char *name)\n {\n \tstruct stat st;\n \treturn stat(rr_path(name, \"preimage\"), &st) ? (time_t) 0 : st.st_mtime;\n }\n \n-static int has_resolution(const char *name)\n-{\n-\tstruct stat st;\n-\treturn !stat(rr_path(name, \"postimage\"), &st);\n-}\n-\n static void unlink_rr_item(const char *name)\n {\n \tunlink(rr_path(name, \"thisimage\"));\ndiff --git a/rerere.c b/rerere.c\nindex 3518207..ac89b80 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -12,12 +12,12 @@ static int rerere_autoupdate;\n \n static char *merge_rr_path;\n \n-static const char *rr_path(const char *name, const char *file)\n+const char *rr_path(const char *name, const char *file)\n {\n \treturn git_path(\"rr-cache/%s/%s\", name, file);\n }\n \n-static int has_resolution(const char *name)\n+int has_resolution(const char *name)\n {\n \tstruct stat st;\n \treturn !stat(rr_path(name, \"postimage\"), &st);\ndiff --git a/rerere.h b/rerere.h\nindex f9b0386..45b5087 100644\n--- a/rerere.h\n+++ b/rerere.h\n@@ -5,5 +5,7 @@\n \n extern int setup_rerere(struct string_list *);\n extern int rerere(void);\n+extern const char *rr_path(const char *name, const char *file);\n+extern int has_resolution(const char *name);\n \n #endif\n-- \n1.6.2.rc0.111.g246ed\n"},{"id":"104657","messageId":"alpine.DEB.1.00.0902141944030.10279@pacific.mpi-cbg.de","threadId":"17789","inReplyTo":"1234628284-4246-1-git-send-email-szeder@ira.uka.de","subject":"Re: [PATCH] rerere: remove duplicated functions","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-14T18:47:36Z","receivedAt":"2009-02-14T18:47:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 14 Feb 2009, SZEDER Gábor wrote:\n\n> diff --git a/rerere.h b/rerere.h\n> index f9b0386..45b5087 100644\n> --- a/rerere.h\n> +++ b/rerere.h\n> @@ -5,5 +5,7 @@\n>  \n>  extern int setup_rerere(struct string_list *);\n>  extern int rerere(void);\n> +extern const char *rr_path(const char *name, const char *file);\n> +extern int has_resolution(const char *name);\n\nYou might argue that rr_path() is a unique enough name (although I'd \nrather name it rerere_path() instead), but has_resolution() is not a good \ndescription of what the function does when it lives in the global name \nspace.  has_rerere_resolution() (with \"hex\" instead of \"name\", to make it \nobvious what the parameter actually _is_) should be a much better name.\n\nThanks,\nDscho\n"},{"id":"104708","messageId":"1234650064-639-1-git-send-email-szeder@ira.uka.de","threadId":"17789","inReplyTo":"alpine.DEB.1.00.0902141944030.10279@pacific.mpi-cbg.de","subject":"[PATCH] rerere: remove duplicated functions","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2009-02-14T22:21:04Z","receivedAt":"2009-02-14T22:21:04Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Both rerere.c and builtin-rerere.c define the static functions\nrr_path() and has_resolution() the exact same way.  To eliminate this\ncode duplication this patch turns the functions in rerere.c\nnon-static, and makes builtin-rerere.c use them.  Also, since this\nputs these two functions into the global namespace, rename them to\nrerere_path() and has_rerere_resolution(), respectively, and rename\ntheir \"name\" parameter to \"hex\", because it better reflects what that\nparameter actually is.\n\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\n\n  On Sat, Feb 14, 2009 at 07:47:36PM +0100, Johannes Schindelin wrote:\n  > > +extern const char *rr_path(const char *name, const char *file);\n  > > +extern int has_resolution(const char *name);\n  > \n  > You might argue that rr_path() is a unique enough name (although I'd\n  > rather name it rerere_path() instead), but has_resolution() is not a\n  > good description of what the function does when it lives in the global\n  > name space.  has_rerere_resolution() (with \"hex\" instead of \"name\", to\n  > make it obvious what the parameter actually _is_) should be a much\n  > better name.\n\n  You are right;  I was aiming for a minimal change and haven't noticed\n  the naming issue.  Renamed both functions and their \"name\" parameter,\n  as you suggested.\n\n\n builtin-rerere.c |   25 +++++++------------------\n rerere.c         |   22 +++++++++++-----------\n rerere.h         |    2 ++\n 3 files changed, 20 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin-rerere.c b/builtin-rerere.c\nindex bd8fc77..020af73 100644\n--- a/builtin-rerere.c\n+++ b/builtin-rerere.c\n@@ -13,28 +13,17 @@ static const char git_rerere_usage[] =\n static int cutoff_noresolve = 15;\n static int cutoff_resolve = 60;\n \n-static const char *rr_path(const char *name, const char *file)\n-{\n-\treturn git_path(\"rr-cache/%s/%s\", name, file);\n-}\n-\n static time_t rerere_created_at(const char *name)\n {\n \tstruct stat st;\n-\treturn stat(rr_path(name, \"preimage\"), &st) ? (time_t) 0 : st.st_mtime;\n-}\n-\n-static int has_resolution(const char *name)\n-{\n-\tstruct stat st;\n-\treturn !stat(rr_path(name, \"postimage\"), &st);\n+\treturn stat(rerere_path(name, \"preimage\"), &st) ? (time_t) 0 : st.st_mtime;\n }\n \n static void unlink_rr_item(const char *name)\n {\n-\tunlink(rr_path(name, \"thisimage\"));\n-\tunlink(rr_path(name, \"preimage\"));\n-\tunlink(rr_path(name, \"postimage\"));\n+\tunlink(rerere_path(name, \"thisimage\"));\n+\tunlink(rerere_path(name, \"preimage\"));\n+\tunlink(rerere_path(name, \"postimage\"));\n \trmdir(git_path(\"rr-cache/%s\", name));\n }\n \n@@ -65,7 +54,7 @@ static void garbage_collect(struct string_list *rr)\n \t\tthen = rerere_created_at(e->d_name);\n \t\tif (!then)\n \t\t\tcontinue;\n-\t\tcutoff = (has_resolution(e->d_name)\n+\t\tcutoff = (has_rerere_resolution(e->d_name)\n \t\t\t  ? cutoff_resolve : cutoff_noresolve);\n \t\tif (then < now - cutoff * 86400)\n \t\t\tstring_list_append(e->d_name, &to_remove);\n@@ -124,7 +113,7 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(argv[1], \"clear\")) {\n \t\tfor (i = 0; i < merge_rr.nr; i++) {\n \t\t\tconst char *name = (const char *)merge_rr.items[i].util;\n-\t\t\tif (!has_resolution(name))\n+\t\t\tif (!has_rerere_resolution(name))\n \t\t\t\tunlink_rr_item(name);\n \t\t}\n \t\tunlink(git_path(\"rr-cache/MERGE_RR\"));\n@@ -137,7 +126,7 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n \t\tfor (i = 0; i < merge_rr.nr; i++) {\n \t\t\tconst char *path = merge_rr.items[i].string;\n \t\t\tconst char *name = (const char *)merge_rr.items[i].util;\n-\t\t\tdiff_two(rr_path(name, \"preimage\"), path, path, path);\n+\t\t\tdiff_two(rerere_path(name, \"preimage\"), path, path, path);\n \t\t}\n \telse\n \t\tusage(git_rerere_usage);\ndiff --git a/rerere.c b/rerere.c\nindex 3518207..713c6e1 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -12,15 +12,15 @@ static int rerere_autoupdate;\n \n static char *merge_rr_path;\n \n-static const char *rr_path(const char *name, const char *file)\n+const char *rerere_path(const char *hex, const char *file)\n {\n-\treturn git_path(\"rr-cache/%s/%s\", name, file);\n+\treturn git_path(\"rr-cache/%s/%s\", hex, file);\n }\n \n-static int has_resolution(const char *name)\n+int has_rerere_resolution(const char *hex)\n {\n \tstruct stat st;\n-\treturn !stat(rr_path(name, \"postimage\"), &st);\n+\treturn !stat(rerere_path(hex, \"postimage\"), &st);\n }\n \n static void read_rr(struct string_list *rr)\n@@ -208,12 +208,12 @@ static int merge(const char *name, const char *path)\n \tmmbuffer_t result = {NULL, 0};\n \txpparam_t xpp = {XDF_NEED_MINIMAL};\n \n-\tif (handle_file(path, NULL, rr_path(name, \"thisimage\")) < 0)\n+\tif (handle_file(path, NULL, rerere_path(name, \"thisimage\")) < 0)\n \t\treturn 1;\n \n-\tif (read_mmfile(&cur, rr_path(name, \"thisimage\")) ||\n-\t\t\tread_mmfile(&base, rr_path(name, \"preimage\")) ||\n-\t\t\tread_mmfile(&other, rr_path(name, \"postimage\")))\n+\tif (read_mmfile(&cur, rerere_path(name, \"thisimage\")) ||\n+\t\t\tread_mmfile(&base, rerere_path(name, \"preimage\")) ||\n+\t\t\tread_mmfile(&other, rerere_path(name, \"postimage\")))\n \t\treturn 1;\n \tret = xdl_merge(&base, &cur, \"\", &other, \"\",\n \t\t\t&xpp, XDL_MERGE_ZEALOUS, &result);\n@@ -291,7 +291,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\t\tstring_list_insert(path, rr)->util = hex;\n \t\t\tif (mkdir(git_path(\"rr-cache/%s\", hex), 0755))\n \t\t\t\tcontinue;\n-\t\t\thandle_file(path, NULL, rr_path(hex, \"preimage\"));\n+\t\t\thandle_file(path, NULL, rerere_path(hex, \"preimage\"));\n \t\t\tfprintf(stderr, \"Recorded preimage for '%s'\\n\", path);\n \t\t}\n \t}\n@@ -307,7 +307,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\tconst char *path = rr->items[i].string;\n \t\tconst char *name = (const char *)rr->items[i].util;\n \n-\t\tif (has_resolution(name)) {\n+\t\tif (has_rerere_resolution(name)) {\n \t\t\tif (!merge(name, path)) {\n \t\t\t\tif (rerere_autoupdate)\n \t\t\t\t\tstring_list_insert(path, &update);\n@@ -326,7 +326,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\t\tcontinue;\n \n \t\tfprintf(stderr, \"Recorded resolution for '%s'.\\n\", path);\n-\t\tcopy_file(rr_path(name, \"postimage\"), path, 0666);\n+\t\tcopy_file(rerere_path(name, \"postimage\"), path, 0666);\n \tmark_resolved:\n \t\trr->items[i].util = NULL;\n \t}\ndiff --git a/rerere.h b/rerere.h\nindex f9b0386..13313f3 100644\n--- a/rerere.h\n+++ b/rerere.h\n@@ -5,5 +5,7 @@\n \n extern int setup_rerere(struct string_list *);\n extern int rerere(void);\n+extern const char *rerere_path(const char *hex, const char *file);\n+extern int has_rerere_resolution(const char *hex);\n \n #endif\n-- \n1.6.2.rc0.111.g246ed\n"},{"id":"104709","messageId":"alpine.DEB.1.00.0902142327520.10279@pacific.mpi-cbg.de","threadId":"17789","inReplyTo":"1234650064-639-1-git-send-email-szeder@ira.uka.de","subject":"Re: [PATCH] rerere: remove duplicated functions","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-14T22:28:30Z","receivedAt":"2009-02-14T22:28:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 14 Feb 2009, SZEDER Gábor wrote:\n\n> Both rerere.c and builtin-rerere.c define the static functions\n> rr_path() and has_resolution() the exact same way.  To eliminate this\n> code duplication this patch turns the functions in rerere.c\n> non-static, and makes builtin-rerere.c use them.  Also, since this\n> puts these two functions into the global namespace, rename them to\n> rerere_path() and has_rerere_resolution(), respectively, and rename\n> their \"name\" parameter to \"hex\", because it better reflects what that\n> parameter actually is.\n\nThanks.  As I started that code in C, I dare say \"ACK\" here :-)\n\nCiao,\nDscho"},{"id":"104803","messageId":"7vk57rlckm.fsf@gitster.siamese.dyndns.org","threadId":"17789","inReplyTo":"alpine.DEB.1.00.0902142327520.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH] rerere: remove duplicated functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-15T19:33:45Z","receivedAt":"2009-02-15T19:33:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Thanks.  As I started that code in C, I dare say \"ACK\" here :-)\n\nThanks, both.\n"}]}