Re: [PATCH 1/2] Add support for a shared config file
From: Thiago Farina <hidden>
Date: 2016-06-15 22:50:07
Some minor nits (style) below. On Thu, Nov 25, 2010 at 3:21 PM, Nathan W. Panike [off-list ref] wrote:
quoted hunk ↗ jump to hunk
The idea is that a project develops indigenous aliases that should be shared project-wide. The only way to communicate this now is by channels outside of git--email or IRC or such. We add support for the case where a project configuration can be in .gitconfig in the top level of the repository. Signed-off-by: Nathan W. Panike <redacted> --- builtin/config.c | 9 ++++++++- config.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 2 deletions(-)diff --git a/builtin/config.c b/builtin/config.c index ca4a0db..26c679d 100644 --- a/builtin/config.c +++ b/builtin/config.c@@ -154,7 +154,7 @@ static int get_value(const char *key_, const char *regex_){ int ret = -1; char *tl; - char *global = NULL, *repo_config = NULL; + char *global = NULL, *repo_config = NULL, *shared = NULL; const char *system_wide = NULL, *local; local = config_exclusive_filename;@@ -165,6 +165,8 @@ static int get_value(const char *key_, const char *regex_)global = xstrdup(mkpath("%s/.gitconfig", home)); if (git_config_system()) system_wide = git_etc_gitconfig(); + if(git_config_shared()) + shared=xstrdup(".gitconfig");
nit: add a space before and after the =
quoted hunk ↗ jump to hunk
} key = xstrdup(key_);@@ -198,11 +200,15 @@ static int get_value(const char *key_, const char *regex_)git_config_from_file(show_config, system_wide, NULL); if (do_all && global) git_config_from_file(show_config, global, NULL); + if (do_all && shared) + git_config_from_shared_file(show_config, shared, NULL); if (do_all) git_config_from_file(show_config, local, NULL); git_config_from_parameters(show_config, NULL); if (!do_all && !seen) git_config_from_file(show_config, local, NULL); + if (!do_all && !seen && shared) + git_config_from_shared_file(show_config, shared, NULL); if (!do_all && !seen && global) git_config_from_file(show_config, global, NULL); if (!do_all && !seen && system_wide)@@ -222,6 +228,7 @@ static int get_value(const char *key_, const char *regex_)free_strings: free(repo_config); free(global); + free(shared); return ret; }diff --git a/config.c b/config.c index c63d683..a9e6bec 100644 --- a/config.c +++ b/config.c@@ -795,6 +795,38 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)return ret; } +struct config_interceptor { + config_fn_t fn; + void* data; +}; +
Would be good to add a description about what is this structure and what is for?
+/*
+ * The purpose of this function is to keep a malicious contributor from
+ * poisoning our configuration. The idea of a shared configuration it to
+ * pass around helpful stuff like aliases, but we do not want to allow someone
+ * to say, change our email address or the url of the remote.
+ */
+
+static int config_interceptor_fn(const char*name, const char* value, void* data)
+{
+ int ret=0;nit: add space before and after the =
+ struct config_interceptor *ci = (struct config_interceptor*)data;
+ if( !ci ) {wrong, wrong.
I think the style used here is:
if (!ci) {
So, add a space after the if, and no spaces in the () please.
+ return -1; + } + if(!prefixcmp(name,"alias."))
same thing here about the space after the if
+ ret = (*ci->fn)(name,value,ci->data);
+ return ret;
+}
+
+int git_config_from_shared_file(config_fn_t fn,const char* filename, void* data)
+{
+ struct config_interceptor ci;
+ ci.fn=fn;nit: fix this too and the other occurrences below.