* [PATCH] environment.c: introduce SETUP_GIT_ENV helper macro
@ 2016-02-27 17:13 Alexander Kuleshov
2016-02-27 17:51 ` Junio C Hamano
0 siblings, 1 reply; 4+ messages in thread
From: Alexander Kuleshov @ 2016-02-27 17:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git, Alexander Kuleshov
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 <kuleshovmail@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] environment.c: introduce SETUP_GIT_ENV helper macro
2016-02-27 17:13 [PATCH] environment.c: introduce SETUP_GIT_ENV helper macro Alexander Kuleshov
@ 2016-02-27 17:51 ` Junio C Hamano
2016-02-27 18:50 ` Junio C Hamano
0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2016-02-27 17:51 UTC (permalink / raw)
To: Alexander Kuleshov; +Cc: Git
Alexander Kuleshov <kuleshovmail@gmail.com> 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 <kuleshovmail@gmail.com>
> ---
> 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)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] environment.c: introduce SETUP_GIT_ENV helper macro
2016-02-27 17:51 ` Junio C Hamano
@ 2016-02-27 18:50 ` Junio C Hamano
2016-02-27 19:10 ` Alexander Kuleshov
0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2016-02-27 18:50 UTC (permalink / raw)
To: Alexander Kuleshov; +Cc: Git
Junio C Hamano <gitster@pobox.com> writes:
> Alexander Kuleshov <kuleshovmail@gmail.com> 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)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] environment.c: introduce SETUP_GIT_ENV helper macro
2016-02-27 18:50 ` Junio C Hamano
@ 2016-02-27 19:10 ` Alexander Kuleshov
0 siblings, 0 replies; 4+ messages in thread
From: Alexander Kuleshov @ 2016-02-27 19:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git
Hello Junio,
On Sun, Feb 28, 2016 at 12:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> 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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-02-27 19:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-27 17:13 [PATCH] environment.c: introduce SETUP_GIT_ENV helper macro Alexander Kuleshov
2016-02-27 17:51 ` Junio C Hamano
2016-02-27 18:50 ` Junio C Hamano
2016-02-27 19:10 ` Alexander Kuleshov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).