Re: [PATCH 1/4] Split up default "core" config parsing into helper routine

From: Jeff King <peff@peff.net>
Date: Jun 18, 2008, 22:49 UTC
Message-ID: <20080618224919.GA22599@sigill.intra.peff.net>
In-Reply-To: <alpine.LFD.1.10.0806181529570.2907@woody.linux-foundation.org>

On Wed, Jun 18, 2008 at 03:30:35PM -0700, Linus Torvalds wrote:

> It makes the code a bit easier to read, and in theory a bit faster too
> (no need to compare all the different "core.*" strings against non-core
> config options).

Maybe it would be easier still to read (and unmeasurably more efficient) to actually do it like:

        if (!prefixcmp(var, "core."))
                return git_default_core_config(var+5, value);
...
int git_default_core_config(const char *var, const char *value)
{
        if (!strcmp(var, "pager"))
                ...

> The config system really should get something of a complete overhaul,
> but in the absense of that, this at least improves on it a tiny bit.

I was curious a while ago and instrumented git_config to write the PID to a tempfile each time it was called. Most git programs parse the config files (.git/config, ~/.gitconfig, /etc/gitconfig) three times each, with some doing it as many as five times.

Most of the config functions are simply "if this key, then set this value". I wonder if it would be simpler to just load the whole thing at once, using a table similar to parseopt.

Then we could do useful things like say "you specified core.foobar, but there is no such variable." I know we can't know all values, since some non-git programs put values in the config, but I don't think it's unreasonable for us to claim all of core.*, especially if it helps us catch simple configuration errors.

-Peff