Some minor nits (style) below. On Thu, Nov 25, 2010 at 3:21 PM, Nathan W. Panike <nathan.panike@xxxxxxxxx> wrote: > 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 <nathan.panike@xxxxxxxxx> > --- > Â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 = >    Â} > >    Â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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html