Re: [PATCH 01/15] string_list: add string_list_duplicate

From: Stefan Beller <sbeller@google.com>
Date: Apr 27, 2016, 21:17 UTC
Message-ID: <CAGZ79kagX5TJU_mbjpo4PKJDoc1wh24DhyS814Kkq76EU9aykA@mail.gmail.com>
In-Reply-To: <xmqqzise7o4l.fsf@gitster.mtv.corp.google.com>

On Wed, Apr 27, 2016 at 2:11 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 24 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> When not duplicating git_config_get_value_multi("submodule.defaultGroup");
>> I run into
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> ...
>> So the string list seems to be corrupted here.
>> Someone stomping over our memory? How long is the result
>> of git_config_get_value_multi deemed to be stable and usable?
>
> That call goes to
>
>     git_config_get_value_multi()
>     -> git_configset_get_value_multi()
>        -> configset_find_element()
>
> the returned value from there would be either NULL or the list of
> values that belong to the config cache layer, i.e. a caller of the
> API can peek but is not allowed to modify it.
>
> So if you are modifying the value you obtain from the API, you must
> make a copy; otherwise you will "stomp over" memory that belongs to
> the config cache layer, not to you.

Yes, we do not modify the string_list (the return of git_config_get_value_multi)

Another way to corrupt it is to change the configuration (e.g. add things to the config hashmap such that it reallocates and grows).

The problem arises after a call to submodule_from_path(...); which may change the cache for submodules. I assumed that was completely different from the regular config cache, but somehow there is a relation, which I have not tracked down yet.

Thanks, Stefan