On Tue, Jun 17, 2014 at 11:51:35AM -0700, Tanay Abhra wrote:
> I have read your other two replies related to it. I suggest the following
> approach
> for git_config_get_string(), it will return,
>
> 1. Return null if no value was found for the entered key.
>
> 2. Empty string (""), returned for NULL values denoting boolean true in some
> cases.
> I think it would be much better than converting NULL to "true" or
> something else
> internally in the function.
> We can easily handle such cases as the above with a strcmp like,
>
> + v = git_config_get_string(alias_key);
> + if (!strcmp(v, ""))
> + config_error_nonbool(alias_key);
>
> What do you think about this approach?
Hmm. Then we can't distinguish between:
[alias]
foo
and
[alias]
foo =
I cannot think of a good reason to do the latter with aliases, but I
wonder if there are other config values for which "the empty string" is
a useful value.
I think I'd lean towards an interface which can actually express the
difference between "key not found" (1), "key present but no value" (2), and "key
present with some string" (3).
You could express (2) with a special pointer value, like:
v = git_config_get_string(key);
if (!v)
return NULL; /* no key */
else if (v == CONFIG_EMPTY_VALUE) {
config_error_nonbool(key);
return NULL;
} else
return xstrdup(v); /* actual value */
It's reasonably concise, but it's a little ugly that if you dereference
CONFIG_EMPTY_VALUE, you get random memory (on the other hand, we could
point it at 0x1, which would make it no different than NULL; you'd get a
segfault in either case).
Another option is to indicate (1) with a return value, and use a
separate parameter for the value. Like:
if (git_config_get_string(key, &v) < 0)
return NULL; /* no key */
else if (!v) {
config_error_nonbool(key); /* no value */
return NULL;
} else
return xstrdup(v); /* actual value */
> Thanks for the suggestion, I was pulling my hair out due this bug for last
> two days.
You're welcome. This is exactly why we encourage students to get their
patches onto the list early for review. There is a lot of git expertise
available on the list. :)
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
