Re: [RFC/PATCH 0/4] config include directives

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 26, 2012 at 08:35, Jeff King <peff@xxxxxxxx> wrote:
> This series provides a way for config files to include other config
> files in two ways:
>
>  1. From other files in the filesystem. This is implemented by patch 1
>     below, and is hopefully straightforward and uncontroversial.  See
>     that patch for more rationale.
>
>  2. From blobs in the repo. This is implemented by patch 4, with
>     patches 2 and 3 providing the necessary refactoring. This
>     is one way of implementing the often asked-for "respect shared
>     config inside the repo" feature, but attempts to mitigate some of
>     the security concerns. The interface for using it safely is a bit
>     raw, but I think it's a sane building block, and somebody could
>     write a fancier shared-config updater on top of it if they wanted
>     to.
>
>  [1/4]: config: add include directive
>  [2/4]: config: factor out config file stack management
>  [3/4]: config: support parsing config data from buffers
>  [4/4]: config: allow including config from repository blobs

I expect you've thought about this, but our current API is (from
add.c):

	git_config(add_config, NULL);

Followed by:

    static int add_config(const char *var, const char *value, void *cb)
    {
    	if (!strcmp(var, "add.ignoreerrors") ||
    	    !strcmp(var, "add.ignore-errors")) {
    		ignore_add_errors = git_config_bool(var, value);
    		return 0;
    	}
    	return git_default_config(var, value, cb);
    }

I.e. that function gets called with one key at a time, and stashes it
to a local value.

If you write the function like that it means your patch series just
works since values encountered later will override earlier ones, but
have you checked git's code to make sure we don't have anything like:

    static int ignore_add_errors_is_set = 0;
    static int add_config(const char *var, const char *value, void *cb)
    {
    	if (!ignore_add_errors_is_set &&
            (!strcmp(var, "add.ignoreerrors") ||
    	     !strcmp(var, "add.ignore-errors"))) {
    		ignore_add_errors = git_config_bool(var, value);
            ignore_add_errors_is_set = 1;
    		return 0;
    	}
    	return git_default_config(var, value, cb);
    }

Which would mean that the include config support would be silently
ignored.
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]