Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> On Thu, Jul 21 2022, Junio C Hamano wrote:
>
>>> +static int clone_submodule(struct module_clone_data *clone_data)
>>> +{
>>> + char *p;
>>> + char *sm_gitdir = clone_submodule_sm_gitdir(clone_data->name);
>>> + char *sm_alternate = NULL, *error_strategy = NULL;
>>> + struct child_process cp = CHILD_PROCESS_INIT;
>>>
>>> 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);
>>
>> This looks like a roundabout way to say xstrfmt().
>
> Yes, I can fix this and others while I'm at it, but a lot of things like
> that in this code are funny uses of APIs that we could improve.
>
> I think it's probably best to just leave these for now.
Yes, this series is already pretty long. For reviewability, perhaps we
could to keep this one focused on "leaks fixes" and leave the style
fixes and refactoring for another series (which might include things
like [1]). As a bonus, with this series in place, we'll know that our
refactoring won't introduce more leaks :)
[1] https://lore.kernel.org/git/kl6lbktitf6e.fsf@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> But I also don't mind adding another commit to this already large series
> to search/replace the relevant strbuf_detach() with xstrfmt()....
>
> Just let me know...
