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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to