[PATCH] environment.c: introduce DECLARE_GIT_GETTER helper macro

Alexander Kuleshov· Feb 27, 2016, 19:35 UTC · lore

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 declaration of these functions to the DECLARE_GIT_GETTER helper macro to prevent code duplication.

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 environment.c | 49 ++++++++++++++-----------------------------------
 1 file changed, 14 insertions(+), 35 deletions(-)

Show changes to environment.c +14 −35

diff --git a/environment.c b/environment.c
index 6dec9d0..f10fc7a 100644
--- a/environment.c
+++ b/environment.c
@@ -126,6 +126,14 @@ const char * const local_repo_env[] = {
 	NULL
 };
 
+#define DECLARE_GIT_GETTER(type, name, env)	\
+	type name(void)				\
+	{					\
+		if (!env)			\
+			setup_git_env();	\
+		return env;			\
+	}
+
 static char *expand_namespace(const char *raw_namespace)
 {
 	struct strbuf buf = STRBUF_INIT;
@@ -197,25 +205,11 @@ int is_bare_repository(void)
 	return is_bare_repository_cfg && !get_git_work_tree();
 }
 
-const char *get_git_dir(void)
-{
-	if (!git_dir)
-		setup_git_env();
-	return git_dir;
-}
-
 const char *get_git_common_dir(void)
 {
 	return git_common_dir;
 }
 
-const char *get_git_namespace(void)
-{
-	if (!namespace)
-		setup_git_env();
-	return namespace;
-}
-
 const char *strip_namespace(const char *namespaced_ref)
 {
 	if (!starts_with(namespaced_ref, get_git_namespace()))
@@ -249,13 +243,6 @@ const char *get_git_work_tree(void)
 	return work_tree;
 }
 
-char *get_object_directory(void)
-{
-	if (!git_object_dir)
-		setup_git_env();
-	return git_object_dir;
-}
-
 int odb_mkstemp(char *template, size_t limit, const char *pattern)
 {
 	int fd;
@@ -293,20 +280,6 @@ int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1)
 	return open(name, O_RDWR|O_CREAT|O_EXCL, 0600);
 }
 
-char *get_index_file(void)
-{
-	if (!git_index_file)
-		setup_git_env();
-	return git_index_file;
-}
-
-char *get_graft_file(void)
-{
-	if (!git_graft_file)
-		setup_git_env();
-	return git_graft_file;
-}
-
 int set_git_dir(const char *path)
 {
 	if (setenv(GIT_DIR_ENVIRONMENT, path, 1))
@@ -325,3 +298,9 @@ const char *get_commit_output_encoding(void)
 {
 	return git_commit_encoding ? git_commit_encoding : "UTF-8";
 }
+
+DECLARE_GIT_GETTER(const char *, get_git_dir, git_dir)
+DECLARE_GIT_GETTER(const char *, get_git_namespace, namespace)
+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)

-- 
2.8.0.rc0.142.g64f2103.dirty

Jeff King· Mar 1, 2016, 15:05 UTC · re: Alexander Kuleshov · lore

Re: [PATCH] environment.c: introduce DECLARE_GIT_GETTER helper macro

On Sun, Feb 28, 2016 at 01:35:44AM +0600, Alexander Kuleshov wrote:

Show 5 quoted lines

> +DECLARE_GIT_GETTER(const char *, get_git_dir, git_dir)
> +DECLARE_GIT_GETTER(const char *, get_git_namespace, namespace)
> +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)

Hmm. I'm somewhat lukewarm on this patch. It's fewer lines and less duplication, which is nice, but this kind of code generation often makes things annoying (to step into with the debugger, to find with ctags, etc). I dunno.

-Peff

Junio C Hamano· Mar 1, 2016, 18:14 UTC · re: Jeff King · lore

Re: [PATCH] environment.c: introduce DECLARE_GIT_GETTER helper macro

Jeff King <peff@peff.net> writes:

Show 12 quoted lines

> On Sun, Feb 28, 2016 at 01:35:44AM +0600, Alexander Kuleshov wrote:
>
>> +DECLARE_GIT_GETTER(const char *, get_git_dir, git_dir)
>> +DECLARE_GIT_GETTER(const char *, get_git_namespace, namespace)
>> +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)
>
> Hmm. I'm somewhat lukewarm on this patch. It's fewer lines and less
> duplication, which is nice, but this kind of code generation often makes
> things annoying (to step into with the debugger, to find with ctags,
> etc). I dunno.

For this particular set of functions, single-step-ability would not be a huge issue, but I am not enthused, either, even though these are vastly more palatable than what was originally proposed.

Another minor annoyance is that I expect to see a semicolon after a pair of parentheses that follows a token, but adding one of course would break the compilation.

patchenvironment.c: introduce DECLARE_GIT_GETTER helper macro