On Thu, Nov 15, 2012 at 08:08:49AM -0800, Jeff King wrote:
> 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 = "";
Hmm, actually, we should probably propagate the error (I was thinking
for some reason this was in the listing code, but it is really about
getting a specific variable, and that variable does not have a sane
format. We'll already have printed the non-bool error, so we should
probably die. So more like:
if (git_config_pathname(&vptr, key_, value_) < 0)
return -1;
must_free_vptr = 1;
-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