On Tue, Nov 13, 2012 at 08:50:04PM -0800, Carlos Martín Nieto wrote:
> When given a variable without a value, such as '[section] var' and
> asking git-config to treat it as a path, git_config_pathname returns
> an error and doesn't modify its output parameter. show_config assumes
> that the call is always successful and sets a variable to indicate
> that vptr should be freed. In case of an error however, trying to do
> this will cause the program to be killed, as it's pointing to memory
> in the stack.
Whoops.
> Set the must_free_vptr flag depending on the return value of
> git_config_pathname so it's accurate.
That is definitely the right thing to do. But do we also need to take
note of the error for later? After this code:
> } else if (types == TYPE_PATH) {
> - git_config_pathname(&vptr, key_, value_);
> - must_free_vptr = 1;
> + must_free_vptr = !git_config_pathname(&vptr, key_, value_);
We don't have any clue that nothing got written into vptr. Which means
it still points at the stack buffer "value", which contains
uninitialized bytes. We will later try to print it, thinking it has the
expanded path in it.
Do we need something like:
if (!git_config_pathname(&vptr, key_, value_))
must_free_vptr = 1;
else
vptr = "";
?
-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