Re: [PATCH v3 04/26] submodule--helper: fix a leak in "clone_submodule"
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jul 22, 2022, 11:30 UTC
- Message-ID
- <220722.8635ets7q2.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <xmqq35euf92d.fsf@gitster.g>
On Thu, Jul 21 2022, Junio C Hamano wrote:
Show 27 quoted lines
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
>> index 7d5ee6a6261..1ddce8e19c1 100644
>> --- a/builtin/submodule--helper.c
>> +++ b/builtin/submodule--helper.c
>> @@ -1658,14 +1658,15 @@ static int clone_submodule(const struct module_clone_data *clone_data,
>> char *sm_alternate = NULL, *error_strategy = NULL;
>> struct child_process cp = CHILD_PROCESS_INIT;
>> const char *clone_data_path;
>> + char *to_free = NULL;
>>
>> if (!is_absolute_path(clone_data->path)) {
>> struct strbuf sb = STRBUF_INIT;
>>
>> strbuf_addf(&sb, "%s/%s", get_git_work_tree(), clone_data->path);
>> - clone_data_path = strbuf_detach(&sb, NULL);
>> + clone_data_path = to_free = strbuf_detach(&sb, NULL);
>> } else {
>> - clone_data_path = xstrdup(clone_data_path);
>> + clone_data_path = clone_data->path;
>> }
>
> Heh, the bug I noticed in the previous step is silently fixed here.
>
> This is why I do not trust a series that is artificially split into
> steps and sent out without self reviewing or even compiling them.Sorry for that mistake, will fix it in a re-roll. It was a result of dumb search-replacing.
But FWIW I did (and do generally) test my changes with "git rebase -i -x make..." before sending them out.
But as you note this doesn't seem to have test coverage, and which gcc yells at me about this, clang (13.0.1-3+b2) does not.
I generally test my changes with both gcc & clang before sending them out, but usually only use clang for the incremental compiling, I'll switch back to gcc...