Re: [PATCHv4 8/8] clone: recursive and reference option triggers submodule alternates
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 17, 2016, 21:31 UTC
- Message-ID
- <xmqqd1l784lf.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <xmqqr39n88pg.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 24 quoted lines
> Stefan Beller <sbeller@google.com> writes:
>
>> +static void prepare_possible_alternates(const char *sm_name,
>> + struct string_list *reference)
>> +{
>> + char *sm_alternate = NULL, *error_strategy = NULL;
>> + struct submodule_alternate_setup sas = SUBMODULE_ALTERNATE_SETUP_INIT;
>> +
>> + git_config_get_string("submodule.alternateLocation", &sm_alternate);
>> + if (!sm_alternate)
>> + return;
>> +
>> + git_config_get_string("submodule.alternateErrorStrategy", &error_strategy);
>
> I have to admit that I need to follow the codepath in config.c every
> time to check, but I _think_ git_config_get_string() gives you your
> own copy of the value. As this function does not give ownership of
> sm_alternate or error_strategy to something else, they are leaked
> every time this function is called, I think.
>
>> + sas.submodule_name = sm_name;
>> + sas.reference = reference;
>> + if (!strcmp(error_strategy, "die"))
>> + sas.error_mode = SUBMODULE_ALTERNATE_ERROR_DIE;Another thing I noticed but forgot to mention. Can error_strategy be NULL here? We are assuming sm_alternate can be, so I presume that it is sensible to protect against dereferencing a NULL here, too?
Show 5 quoted lines
>> + if (!strcmp(error_strategy, "info")) >> + sas.error_mode = SUBMODULE_ALTERNATE_ERROR_INFO; >> + if (!strcmp(sm_alternate, "superproject")) >> + foreach_alt_odb(add_possible_reference_from_superproject, &sas); >> +}