git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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);
>> +}
Previous: Stefan BellerNext: Stefan Beller
Message 17 of 18 in “[PATCHv4 0/6] git clone: Marry --recursive and --reference”
  1. Stefan BellerAug 11, 2016
  2. 1/8 t7408: modernize styleStefan Beller, Aug 11, 2016
  3. 2/8 t7408: merge short tests, factor out testing methodStefan Beller, Aug 11, 2016
  4. 3/8 submodule--helper module-clone: allow multiple referencesStefan Beller, Aug 11, 2016
  5. 4/8 submodule--helper update-clone: allow multiple referencesStefan Beller, Aug 11, 2016
  6. 5/8 clone: factor out checking for an alternate pathStefan Beller, Aug 11, 2016
  7. Junio C HamanoAug 12, 2016
  8. Stefan BellerAug 15, 2016
  9. Junio C HamanoAug 15, 2016
  10. Stefan BellerAug 15, 2016
  11. 7/8 clone: implement optional referencesStefan Beller, Aug 11, 2016
  12. 6/8 clone: clarify option_reference as requiredStefan Beller, Aug 11, 2016
  13. 8/8 clone: recursive and reference option triggers submodule alternatesStefan Beller, Aug 11, 2016
  14. Junio C HamanoAug 12, 2016
  15. Junio C HamanoAug 17, 2016
  16. Stefan BellerAug 17, 2016
  17. Junio C HamanoAug 17, 2016
  18. Stefan BellerAug 17, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.