Re: [PATCH] Git segmentation faults if submodule path is empty.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 16, 2013 at 08:48:43AM +0200, Jens Lehmann wrote:

> > This patch addresses the issue by returning from the function if
> > 'value' is null before the call to xstrdup is made.
> 
> Hmm, I'm not sure silently ignoring the misconfiguration is the best
> way to go. A submodule config having a path setting without a value
> is broken (while a submodule setting without a subsection configures
> something else, so the "|| !name" below is fine). So I believe we
> should complain to the user when "value" is NULL.

Yeah, the usual behavior is to catch it and return config_error_nonbool
(because a key without a value is an implicit-true boolean).  Most code
does this via git_config_string, but what the submodule code is doing is
too tricky and custom to use that stock function.

> On the other hand this should only happen for the three options we do
> parse, as some users (e.g. git-submodule.sh) use other configurations
> for which a missing value may be fine. Please see the "lacks value"
> errors in read_merge_config() of ll-merge.c for an example of how to
> deal with that.

Those spots in ll-merge should probably be using config_error_nonbool,
if only for consistency with the rest of the code base.

> > -	if (parse_config_key(var, "submodule", &name, &namelen, &key) < 0 || !name)
> > +	if (parse_config_key(var, "submodule", &name, &namelen, &key) < 0 || !name || !value)
> >  		return 0;

I think this is also the wrong place to make the check, anyway. It is
saying that all values of submodule.X.Y must be non-NULL. But that is
not true. The submodule.X.fetchRecurseSubmodules option can be a
boolean, and:

  [submodule "foo"]
    fetchRecurseSubmodules

is perfectly valid (and is broken by this patch).

-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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]