Re: [PATCH] transport: add core.allowProtocol config option

From: Jeff King <peff@peff.net>
Date: Nov 3, 2016, 00:08 UTC
Message-ID: <20161103000839.fwmqqggvmhzo7zmx@sigill.intra.peff.net>
In-Reply-To: <20161102234613.GD183367@google.com>

On Wed, Nov 02, 2016 at 04:46:13PM -0700, Brandon Williams wrote:

Show 18 quoted lines

> > > I thought at first we'd have to deal with leaking "v", but "get_value"
> > > is the "raw" version that gives you the uninterpreted value. I think
> > > that means it may give you NULL, though if we see an implicit bool like:
> > > 
> > >   [core]
> > >   allowProtocol
> > > 
> > > That's nonsense, of course, but we would still segfault. I
> > > think the easiest way to test is:
> > > 
> > >   git -c core.allowProtocol fetch
> > > 
> > > which seems to segfault for me with this patch.
> > 
> > what is the desired behavior when a user provides a config in a way that
> > isn't intended?
> 
> oh...I can just drop in git_config_get_string_const() instead.

Yes, it will call git_config_string(), which will make sure there's an actual value and die otherwise. But note that it also duplicates the string, so you'd have to deal with freeing it.

-Peff