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

[Junio C Hamano]

Jeff King  writes:

> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Jeff King]

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.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] environment.c: introduce DECLARE_GIT_GETTER helper macro

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

Signed-off-by: Alexander Kuleshov 
---
 environment.c | 49 ++---
 1 file changed, 14 insertions(+), 35 deletions(-)

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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html