On Mon, Jun 16, 2014 at 02:15:54AM -0700, Tanay Abhra wrote:
> **DOUBT**
> This patch builds on top of patch series[1]. The first patch in the
> replace `git_config` series is [2], which passed all the tests.
>
> But this patch falters at this test in t1300-repo-config.sh,
>
> git config alias.checkconfig "-c foo.check=bar config foo.check" &&
> echo bar >expect &&
> git checkconfig >actual &&
> test_cmp expect actual
>
> I hand tested this case and the outputs match. But I don't know why the tests
> are failing.
I get:
expecting success:
git config alias.split-cmdline-fix 'echo "' &&
test_must_fail git split-cmdline-fix &&
echo foo > foo &&
git add foo &&
git commit -m 'initial commit' &&
git config branch.master.mergeoptions 'echo "' &&
test_must_fail git merge master
Segmentation fault
test_must_fail: died by signal: git split-cmdline-fix
Running with valgrind gives more details (it looks like the segfault I
mentioned in the other thread).
> char *alias_lookup(const char *alias)
> {
> - alias_key = alias;
> - alias_val = NULL;
> - git_config(alias_lookup_cb, NULL);
> + char *alias_key, *alias_val;
> + const char *v;
> + alias_key = xmalloc(7+strlen(alias));
> + strcpy(alias_key, "alias.");
> + strcat(alias_key, alias);
Please use a strbuf instead of hand-rolling the math. It's much easier
to verify that it is correct, and it avoids badly designed functions
like strcat. I.e.:
struct strbuf key = STRBUF_INIT;
strbuf_addf(&key, "alias.%s", alias);
...
strbuf_release(&key);
(note also that since the key/val variables are no longer static
globals, it's fine to use a shorter, less clunky name).
> + v = git_config_get_string(alias_key);
> + if (!v)
> + config_error_nonbool(alias_key);
What does a NULL output from git_config_get_string mean? I think with
the current code, it means "no such key was found". In which case, you
should be returning NULL here (there is no such alias), not complaining
with config_error_nonbool.
Again, this is going to depend on your strategy for storing booleans
that I mentioned elsewhere.
-Peff
--
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