{"thread":{"id":"41555","subject":"[PATCH] environment.c: introduce DECLARE_GIT_GETTER helper macro","startedAt":"2016-02-27T19:35:44Z","lastAt":"2016-03-01T18:14:05Z","messageCount":3,"participants":["Alexander Kuleshov","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"279670","messageId":"1456601744-18404-1-git-send-email-kuleshovmail@gmail.com","threadId":"41555","inReplyTo":null,"subject":"[PATCH] environment.c: introduce DECLARE_GIT_GETTER helper macro","fromName":"Alexander Kuleshov","fromEmail":"kuleshovmail@gmail.com","sentAt":"2016-02-27T19:35:44Z","receivedAt":"2016-02-27T19:35:44Z","isPatch":true,"sender":{"key":"kuleshovmail@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2699235?v=4"},"body":"The environment.c contans a couple of functions which are\nconsist from the following pattern:\n\n        if (!env)\n                setup_git_env();\n        return env;\n\nLet's move declaration of these functions to the DECLARE_GIT_GETTER\nhelper macro to prevent code duplication.\n\nSigned-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>\n---\n environment.c | 49 ++++++++++++++-----------------------------------\n 1 file changed, 14 insertions(+), 35 deletions(-)\n\ndiff --git a/environment.c b/environment.c\nindex 6dec9d0..f10fc7a 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -126,6 +126,14 @@ const char * const local_repo_env[] = {\n \tNULL\n };\n \n+#define DECLARE_GIT_GETTER(type, name, env)\t\\\n+\ttype name(void)\t\t\t\t\\\n+\t{\t\t\t\t\t\\\n+\t\tif (!env)\t\t\t\\\n+\t\t\tsetup_git_env();\t\\\n+\t\treturn env;\t\t\t\\\n+\t}\n+\n static char *expand_namespace(const char *raw_namespace)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -197,25 +205,11 @@ int is_bare_repository(void)\n \treturn is_bare_repository_cfg && !get_git_work_tree();\n }\n \n-const char *get_git_dir(void)\n-{\n-\tif (!git_dir)\n-\t\tsetup_git_env();\n-\treturn git_dir;\n-}\n-\n const char *get_git_common_dir(void)\n {\n \treturn git_common_dir;\n }\n \n-const char *get_git_namespace(void)\n-{\n-\tif (!namespace)\n-\t\tsetup_git_env();\n-\treturn namespace;\n-}\n-\n const char *strip_namespace(const char *namespaced_ref)\n {\n \tif (!starts_with(namespaced_ref, get_git_namespace()))\n@@ -249,13 +243,6 @@ const char *get_git_work_tree(void)\n \treturn work_tree;\n }\n \n-char *get_object_directory(void)\n-{\n-\tif (!git_object_dir)\n-\t\tsetup_git_env();\n-\treturn git_object_dir;\n-}\n-\n int odb_mkstemp(char *template, size_t limit, const char *pattern)\n {\n \tint fd;\n@@ -293,20 +280,6 @@ int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1)\n \treturn open(name, O_RDWR|O_CREAT|O_EXCL, 0600);\n }\n \n-char *get_index_file(void)\n-{\n-\tif (!git_index_file)\n-\t\tsetup_git_env();\n-\treturn git_index_file;\n-}\n-\n-char *get_graft_file(void)\n-{\n-\tif (!git_graft_file)\n-\t\tsetup_git_env();\n-\treturn git_graft_file;\n-}\n-\n int set_git_dir(const char *path)\n {\n \tif (setenv(GIT_DIR_ENVIRONMENT, path, 1))\n@@ -325,3 +298,9 @@ const char *get_commit_output_encoding(void)\n {\n \treturn git_commit_encoding ? git_commit_encoding : \"UTF-8\";\n }\n+\n+DECLARE_GIT_GETTER(const char *, get_git_dir, git_dir)\n+DECLARE_GIT_GETTER(const char *, get_git_namespace, namespace)\n+DECLARE_GIT_GETTER(char *, get_object_directory, git_object_dir)\n+DECLARE_GIT_GETTER(char *, get_index_file, git_index_file)\n+DECLARE_GIT_GETTER(char *, get_graft_file, git_graft_file)\n-- \n2.8.0.rc0.142.g64f2103.dirty\n"},{"id":"279973","messageId":"20160301150543.GN12887@sigill.intra.peff.net","threadId":"41555","inReplyTo":"1456601744-18404-1-git-send-email-kuleshovmail@gmail.com","subject":"Re: [PATCH] environment.c: introduce DECLARE_GIT_GETTER helper macro","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-01T15:05:43Z","receivedAt":"2016-03-01T15:05:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 28, 2016 at 01:35:44AM +0600, Alexander Kuleshov wrote:\n\n> +DECLARE_GIT_GETTER(const char *, get_git_dir, git_dir)\n> +DECLARE_GIT_GETTER(const char *, get_git_namespace, namespace)\n> +DECLARE_GIT_GETTER(char *, get_object_directory, git_object_dir)\n> +DECLARE_GIT_GETTER(char *, get_index_file, git_index_file)\n> +DECLARE_GIT_GETTER(char *, get_graft_file, git_graft_file)\n\nHmm. I'm somewhat lukewarm on this patch. It's fewer lines and less\nduplication, which is nice, but this kind of code generation often makes\nthings annoying (to step into with the debugger, to find with ctags,\netc). I dunno.\n\n-Peff\n"},{"id":"279990","messageId":"xmqqy4a26p76.fsf@gitster.mtv.corp.google.com","threadId":"41555","inReplyTo":"20160301150543.GN12887@sigill.intra.peff.net","subject":"Re: [PATCH] environment.c: introduce DECLARE_GIT_GETTER helper macro","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-01T18:14:05Z","receivedAt":"2016-03-01T18:14:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sun, Feb 28, 2016 at 01:35:44AM +0600, Alexander Kuleshov wrote:\n>\n>> +DECLARE_GIT_GETTER(const char *, get_git_dir, git_dir)\n>> +DECLARE_GIT_GETTER(const char *, get_git_namespace, namespace)\n>> +DECLARE_GIT_GETTER(char *, get_object_directory, git_object_dir)\n>> +DECLARE_GIT_GETTER(char *, get_index_file, git_index_file)\n>> +DECLARE_GIT_GETTER(char *, get_graft_file, git_graft_file)\n>\n> Hmm. I'm somewhat lukewarm on this patch. It's fewer lines and less\n> duplication, which is nice, but this kind of code generation often makes\n> things annoying (to step into with the debugger, to find with ctags,\n> etc). I dunno.\n\nFor this particular set of functions, single-step-ability would not\nbe a huge issue, but I am not enthused, either, even though these\nare vastly more palatable than what was originally proposed.\n\nAnother minor annoyance is that I expect to see a semicolon after a\npair of parentheses that follows a token, but adding one of course\nwould break the compilation.\n"}]}