# [PATCH] environment.c: introduce SETUP_GIT_ENV helper macro 4 messages from 2016-02-27 to 2016-02-27. Participants: Alexander Kuleshov, Junio C Hamano. Thread: https://gitlist.dev/t/41553 ## Alexander Kuleshov, 2016-02-27 17:13 Subject: [PATCH] environment.c: introduce SETUP_GIT_ENV helper macro Message-ID: <1456593215-16302-1-git-send-email-kuleshovmail@gmail.com> URL: https://gitlist.dev/e/1456593215-16302-1-git-send-email-kuleshovmail%40gmail.com ``` The environment.c contans a couple of functions which are consist from the following pattern: if (!env) setup_git_env(); return env; Let's move this to the SETUP_GIT_ENV helper macro to prevent code duplication in these functions. Signed-off-by: Alexander Kuleshov --- environment.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/environment.c b/environment.c index 6dec9d0..04cb6cd 100644 --- a/environment.c +++ b/environment.c @@ -126,6 +126,11 @@ const char * const local_repo_env[] = { NULL }; +#define SETUP_GIT_ENV(env) \ + if (!env) \ + setup_git_env(); \ + return env; + static char *expand_namespace(const char *raw_namespace) { struct strbuf buf = STRBUF_INIT; @@ -199,9 +204,7 @@ int is_bare_repository(void) const char *get_git_dir(void) { - if (!git_dir) - setup_git_env(); - return git_dir; + SETUP_GIT_ENV(git_dir); } const char *get_git_common_dir(void) @@ -211,9 +214,7 @@ const char *get_git_common_dir(void) const char *get_git_namespace(void) { - if (!namespace) - setup_git_env(); - return namespace; + SETUP_GIT_ENV(namespace); } const char *strip_namespace(const char *namespaced_ref) @@ -251,9 +252,7 @@ const char *get_git_work_tree(void) char *get_object_directory(void) { - if (!git_object_dir) - setup_git_env(); - return git_object_dir; + SETUP_GIT_ENV(git_object_dir); } int odb_mkstemp(char *template, size_t limit, const char *pattern) @@ -295,16 +294,12 @@ int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1) char *get_index_file(void) { - if (!git_index_file) - setup_git_env(); - return git_index_file; + SETUP_GIT_ENV(git_index_file); } char *get_graft_file(void) { - if (!git_graft_file) - setup_git_env(); - return git_graft_file; + SETUP_GIT_ENV(git_graft_file); } int set_git_dir(const char *path) -- 2.5.0 ``` ## Junio C Hamano, 2016-02-27 17:51 Subject: Re: [PATCH] environment.c: introduce SETUP_GIT_ENV helper macro Message-ID: URL: https://gitlist.dev/e/xmqqpovighxk.fsf%40gitster.mtv.corp.google.com In-Reply-To: <1456593215-16302-1-git-send-email-kuleshovmail@gmail.com> ``` Alexander Kuleshov writes: > The environment.c contans a couple of functions which are > consist from the following pattern: > > if (!env) > setup_git_env(); > return env; > > Let's move this to the SETUP_GIT_ENV helper macro to prevent > code duplication in these functions. Please don't. A macro that hides "return" makes things harder to follow, not easier. > > Signed-off-by: Alexander Kuleshov > --- > environment.c | 25 ++++++++++--------------- > 1 file changed, 10 insertions(+), 15 deletions(-) > > diff --git a/environment.c b/environment.c > index 6dec9d0..04cb6cd 100644 > --- a/environment.c > +++ b/environment.c > @@ -126,6 +126,11 @@ const char * const local_repo_env[] = { > NULL > }; > > +#define SETUP_GIT_ENV(env) \ > + if (!env) \ > + setup_git_env(); \ > + return env; > + > static char *expand_namespace(const char *raw_namespace) > { > struct strbuf buf = STRBUF_INIT; > @@ -199,9 +204,7 @@ int is_bare_repository(void) > > const char *get_git_dir(void) > { > - if (!git_dir) > - setup_git_env(); > - return git_dir; > + SETUP_GIT_ENV(git_dir); > } > > const char *get_git_common_dir(void) > @@ -211,9 +214,7 @@ const char *get_git_common_dir(void) > > const char *get_git_namespace(void) > { > - if (!namespace) > - setup_git_env(); > - return namespace; > + SETUP_GIT_ENV(namespace); > } > > const char *strip_namespace(const char *namespaced_ref) > @@ -251,9 +252,7 @@ const char *get_git_work_tree(void) > > char *get_object_directory(void) > { > - if (!git_object_dir) > - setup_git_env(); > - return git_object_dir; > + SETUP_GIT_ENV(git_object_dir); > } > > int odb_mkstemp(char *template, size_t limit, const char *pattern) > @@ -295,16 +294,12 @@ int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1) > > char *get_index_file(void) > { > - if (!git_index_file) > - setup_git_env(); > - return git_index_file; > + SETUP_GIT_ENV(git_index_file); > } > > char *get_graft_file(void) > { > - if (!git_graft_file) > - setup_git_env(); > - return git_graft_file; > + SETUP_GIT_ENV(git_graft_file); > } > > int set_git_dir(const char *path) ``` ## Junio C Hamano, 2016-02-27 18:50 Subject: Re: [PATCH] environment.c: introduce SETUP_GIT_ENV helper macro Message-ID: URL: https://gitlist.dev/e/xmqqsi0ef0mz.fsf%40gitster.mtv.corp.google.com In-Reply-To: ``` Junio C Hamano writes: > Alexander Kuleshov writes: > >> Let's move this to the SETUP_GIT_ENV helper macro to prevent >> code duplication in these functions. > > Please don't. A macro that hides "return" makes things harder to > follow, not easier. >> >> +#define SETUP_GIT_ENV(env) \ >> + if (!env) \ >> + setup_git_env(); \ >> + return env; >> + >> static char *expand_namespace(const char *raw_namespace) >> { >> struct strbuf buf = STRBUF_INIT; >> @@ -199,9 +204,7 @@ int is_bare_repository(void) >> >> const char *get_git_dir(void) >> { >> - if (!git_dir) >> - setup_git_env(); >> - return git_dir; >> + SETUP_GIT_ENV(git_dir); >> } Having said that, I do think a higher-level macro that encapulates the whole thing may not be such a bad idea, i.e. making the above into DECLARE_GIT_GETTER(const char *, get_git_dir, git_dir) and this and others >> char *get_object_directory(void) >> { >> - if (!git_object_dir) >> - setup_git_env(); >> - return git_object_dir; >> + SETUP_GIT_ENV(git_object_dir); >> } into DECLARE_GIT_GETTER(char *, get_object_directory, git_object_dir) DECLARE_GIT_GETTER(char *, get_index_file, git_index_file) DECLARE_GIT_GETTER(char *, get_graft_file, git_graft_file) ``` ## Alexander Kuleshov, 2016-02-27 19:10 Subject: Re: [PATCH] environment.c: introduce SETUP_GIT_ENV helper macro Message-ID: URL: https://gitlist.dev/e/CANCZXo5aLD_9cjzwPK5zw_ZHmevxmX8c_e9ONPyByF7jn_zF7Q%40mail.gmail.com In-Reply-To: ``` Hello Junio, On Sun, Feb 28, 2016 at 12:50 AM, Junio C Hamano wrote: > Junio C Hamano writes: > > >> Please don't. A macro that hides "return" makes things harder to >> follow, not easier. I will consider it next time. > Having said that, I do think a higher-level macro that encapulates > the whole thing may not be such a bad idea, i.e. making the above > into > > DECLARE_GIT_GETTER(const char *, get_git_dir, git_dir) Yes, it is better. Will resend the patch. Thank you. ```