Re: [GSoC] [PATCH v2 6/9] submodule--helper: convert the bulk of cmd_add() to C

From: Atharva Raykar <raykar.ath@gmail.com>
Date: Aug 6, 2021, 11:33 UTC
Message-ID: <m2wnoyu7sf.fsf@gmail.com>
In-Reply-To: <YQyNDv3bzVZwpAVl@danh.dev>

Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:

Show 25 quoted lines

> On 2021-08-05 13:10:51+0530, Atharva Raykar <raykar.ath@gmail.com> wrote:
>> [...]
>> +		add_data.sm_path = xstrdup(argv[1]);
>
> add_data.sm_path is allocated in this block (regardless of legs).
>
>> [...]
>> +		if ((exit_code = pipe_command(&cp, NULL, 0, NULL, 0, &sb, 0))) {
>> +			strbuf_complete_line(&sb);
>> +			fputs(sb.buf, stderr);
>> +			return exit_code;
>
> But, we don't free it when return from here.
>
>> [...]
>> +
>> +	if (add_submodule(&add_data))
>> +		return 1;
>
> And here.
>
>> +	configure_added_submodule(&add_data);
>> +	free(add_data.sm_path);
>
> However, it will be free()-d here, is it intended?

Yeah I meant to have it free()'d wherever possible, although I suppose it isn't strictly necessary since we exit.

Show 5 quoted lines

> I think we may use UNLEAK above (for now) because we will exit process
> after this function.
>
> However, I anticipated we may need to do more stuffs after this
> function in the future.

Right. So it's better I ensure that it's freed properly everywhere.

>> [...]
>> --
>> 2.32.0
>>