{"thread":{"id":"41553","subject":"[PATCH] environment.c: introduce SETUP_GIT_ENV helper macro","startedAt":"2016-02-27T17:13:35Z","lastAt":"2016-02-27T19:10:55Z","messageCount":4,"participants":["Alexander Kuleshov","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"279646","messageId":"1456593215-16302-1-git-send-email-kuleshovmail@gmail.com","threadId":"41553","inReplyTo":null,"subject":"[PATCH] environment.c: introduce SETUP_GIT_ENV helper macro","fromName":"Alexander Kuleshov","fromEmail":"kuleshovmail@gmail.com","sentAt":"2016-02-27T17:13:35Z","receivedAt":"2016-02-27T17:13:35Z","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 this to the SETUP_GIT_ENV helper macro to prevent\ncode duplication in these functions.\n\nSigned-off-by: Alexander Kuleshov \n---\n environment.c | 25 ++++++++++---------------\n 1 file changed, 10 insertions(+), 15 deletions(-)\n\ndiff --git a/environment.c b/environment.c\nindex 6dec9d0..04cb6cd 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -126,6 +126,11 @@ const char * const local_repo_env[] = {\n \tNULL\n };\n \n+#define SETUP_GIT_ENV(env) \\\n+\tif (!env) \\\n+\t\tsetup_git_env(); \\\n+\treturn env;\n+\n static char *expand_namespace(const char *raw_namespace)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -199,9 +204,7 @@ int is_bare_repository(void)\n \n const char *get_git_dir(void)\n {\n-\tif (!git_dir)\n-\t\tsetup_git_env();\n-\treturn git_dir;\n+\tSETUP_GIT_ENV(git_dir);\n }\n \n const char *get_git_common_dir(void)\n@@ -211,9 +214,7 @@ const char *get_git_common_dir(void)\n \n const char *get_git_namespace(void)\n {\n-\tif (!namespace)\n-\t\tsetup_git_env();\n-\treturn namespace;\n+\tSETUP_GIT_ENV(namespace);\n }\n \n const char *strip_namespace(const char *namespaced_ref)\n@@ -251,9 +252,7 @@ const char *get_git_work_tree(void)\n \n char *get_object_directory(void)\n {\n-\tif (!git_object_dir)\n-\t\tsetup_git_env();\n-\treturn git_object_dir;\n+\tSETUP_GIT_ENV(git_object_dir);\n }\n \n int odb_mkstemp(char *template, size_t limit, const char *pattern)\n@@ -295,16 +294,12 @@ int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1)\n \n char *get_index_file(void)\n {\n-\tif (!git_index_file)\n-\t\tsetup_git_env();\n-\treturn git_index_file;\n+\tSETUP_GIT_ENV(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+\tSETUP_GIT_ENV(git_graft_file);\n }\n \n int set_git_dir(const char *path)\n-- \n2.5.0\n"},{"id":"279655","messageId":"xmqqpovighxk.fsf@gitster.mtv.corp.google.com","threadId":"41553","inReplyTo":"1456593215-16302-1-git-send-email-kuleshovmail@gmail.com","subject":"Re: [PATCH] environment.c: introduce SETUP_GIT_ENV helper macro","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-27T17:51:51Z","receivedAt":"2016-02-27T17:51:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexander Kuleshov writes:\n\n> The environment.c contans a couple of functions which are\n> consist from the following pattern:\n>\n> if (!env)\n> setup_git_env();\n> return env;\n>\n> Let's move this to the SETUP_GIT_ENV helper macro to prevent\n> code duplication in these functions.\n\nPlease don't. A macro that hides \"return\" makes things harder to\nfollow, not easier.\n\n>\n> Signed-off-by: Alexander Kuleshov \n> ---\n> environment.c | 25 ++++++++++---------------\n> 1 file changed, 10 insertions(+), 15 deletions(-)\n>\n> diff --git a/environment.c b/environment.c\n> index 6dec9d0..04cb6cd 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -126,6 +126,11 @@ const char * const local_repo_env[] = {\n> \tNULL\n> };\n> \n> +#define SETUP_GIT_ENV(env) \\\n> +\tif (!env) \\\n> +\t\tsetup_git_env(); \\\n> +\treturn env;\n> +\n> static char *expand_namespace(const char *raw_namespace)\n> {\n> \tstruct strbuf buf = STRBUF_INIT;\n> @@ -199,9 +204,7 @@ int is_bare_repository(void)\n> \n> const char *get_git_dir(void)\n> {\n> -\tif (!git_dir)\n> -\t\tsetup_git_env();\n> -\treturn git_dir;\n> +\tSETUP_GIT_ENV(git_dir);\n> }\n> \n> const char *get_git_common_dir(void)\n> @@ -211,9 +214,7 @@ const char *get_git_common_dir(void)\n> \n> const char *get_git_namespace(void)\n> {\n> -\tif (!namespace)\n> -\t\tsetup_git_env();\n> -\treturn namespace;\n> +\tSETUP_GIT_ENV(namespace);\n> }\n> \n> const char *strip_namespace(const char *namespaced_ref)\n> @@ -251,9 +252,7 @@ const char *get_git_work_tree(void)\n> \n> char *get_object_directory(void)\n> {\n> -\tif (!git_object_dir)\n> -\t\tsetup_git_env();\n> -\treturn git_object_dir;\n> +\tSETUP_GIT_ENV(git_object_dir);\n> }\n> \n> int odb_mkstemp(char *template, size_t limit, const char *pattern)\n> @@ -295,16 +294,12 @@ int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1)\n> \n> char *get_index_file(void)\n> {\n> -\tif (!git_index_file)\n> -\t\tsetup_git_env();\n> -\treturn git_index_file;\n> +\tSETUP_GIT_ENV(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> +\tSETUP_GIT_ENV(git_graft_file);\n> }\n> \n> int set_git_dir(const char *path)\n"},{"id":"279663","messageId":"xmqqsi0ef0mz.fsf@gitster.mtv.corp.google.com","threadId":"41553","inReplyTo":"xmqqpovighxk.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] environment.c: introduce SETUP_GIT_ENV helper macro","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-27T18:50:44Z","receivedAt":"2016-02-27T18:50:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Alexander Kuleshov writes:\n>\n>> Let's move this to the SETUP_GIT_ENV helper macro to prevent\n>> code duplication in these functions.\n>\n> Please don't. A macro that hides \"return\" makes things harder to\n> follow, not easier.\n>>\n>> +#define SETUP_GIT_ENV(env) \\\n>> +\tif (!env) \\\n>> +\t\tsetup_git_env(); \\\n>> +\treturn env;\n>> +\n>> static char *expand_namespace(const char *raw_namespace)\n>> {\n>> \tstruct strbuf buf = STRBUF_INIT;\n>> @@ -199,9 +204,7 @@ int is_bare_repository(void)\n>> \n>> const char *get_git_dir(void)\n>> {\n>> -\tif (!git_dir)\n>> -\t\tsetup_git_env();\n>> -\treturn git_dir;\n>> +\tSETUP_GIT_ENV(git_dir);\n>> }\n\nHaving said that, I do think a higher-level macro that encapulates\nthe whole thing may not be such a bad idea, i.e. making the above\ninto\n\n DECLARE_GIT_GETTER(const char *, get_git_dir, git_dir)\n\nand this and others\n\n>> char *get_object_directory(void)\n>> {\n>> -\tif (!git_object_dir)\n>> -\t\tsetup_git_env();\n>> -\treturn git_object_dir;\n>> +\tSETUP_GIT_ENV(git_object_dir);\n>> }\n\ninto\n\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"},{"id":"279666","messageId":"CANCZXo5aLD_9cjzwPK5zw_ZHmevxmX8c_e9ONPyByF7jn_zF7Q@mail.gmail.com","threadId":"41553","inReplyTo":"xmqqsi0ef0mz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] environment.c: introduce SETUP_GIT_ENV helper macro","fromName":"Alexander Kuleshov","fromEmail":"kuleshovmail@gmail.com","sentAt":"2016-02-27T19:10:55Z","receivedAt":"2016-02-27T19:10:55Z","isPatch":true,"sender":{"key":"kuleshovmail@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2699235?v=4"},"body":"Hello Junio,\n\nOn Sun, Feb 28, 2016 at 12:50 AM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>\n>> Please don't. A macro that hides \"return\" makes things harder to\n>> follow, not easier.\n\nI will consider it next time.\n\n> Having said that, I do think a higher-level macro that encapulates\n> the whole thing may not be such a bad idea, i.e. making the above\n> into\n>\n> DECLARE_GIT_GETTER(const char *, get_git_dir, git_dir)\n\nYes, it is better. Will resend the patch.\n\nThank you.\n"}]}