2018-01-10 16:13:51

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 5/6] kconfig: Don't leak help strings during parsing

2017-10-09 2:11 GMT+09:00 Ulf Magnusson <[email protected]>:
> This is just for completeness to get rid of the last memory leak
> currently generated during parsing for ARCH=x86. The symbol
> DVB_NETUP_UNIDVB in drivers/media/pci/netup_unidvb/Kconfig currently has
> two help strings, and we leak the first one.

Looks like it was fixed by
561b29e4ec8d0aac7e094f70d649ee4abccdda03



> Summary from Valgrind on 'menuconfig' (ARCH=x86) before the fix:
>
> LEAK SUMMARY:
> definitely lost: 344,440 bytes in 14,350 blocks
> ...
>
> Summary after the fix:
>
> LEAK SUMMARY:
> definitely lost: 344,376 bytes in 14,349 blocks
> ...
>
> Signed-off-by: Ulf Magnusson <[email protected]>
> ---
> scripts/kconfig/zconf.y | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/scripts/kconfig/zconf.y b/scripts/kconfig/zconf.y
> index 468ab03..3c9f436 100644
> --- a/scripts/kconfig/zconf.y
> +++ b/scripts/kconfig/zconf.y
> @@ -435,6 +435,9 @@ help_start: T_HELP T_EOL
>
> help: help_start T_HELPTEXT
> {
> + if (current_entry->help)
> + /* Weird menu node with two help strings */
> + free(current_entry->help);
> current_entry->help = $2;
> };


I applied this, but maybe is it better to warning message?
zconfprint("warning: ...") or zconf_error()?


If you send v2, I will replace this.




--
Best Regards
Masahiro Yamada


2018-01-12 08:48:00

by Ulf Magnusson

[permalink] [raw]
Subject: Re: [PATCH 5/6] kconfig: Don't leak help strings during parsing

On Wed, Jan 10, 2018 at 5:12 PM, Masahiro Yamada
<[email protected]> wrote:
> 2017-10-09 2:11 GMT+09:00 Ulf Magnusson <[email protected]>:
>> This is just for completeness to get rid of the last memory leak
>> currently generated during parsing for ARCH=x86. The symbol
>> DVB_NETUP_UNIDVB in drivers/media/pci/netup_unidvb/Kconfig currently has
>> two help strings, and we leak the first one.
>
> Looks like it was fixed by
> 561b29e4ec8d0aac7e094f70d649ee4abccdda03
>
>
>
>> Summary from Valgrind on 'menuconfig' (ARCH=x86) before the fix:
>>
>> LEAK SUMMARY:
>> definitely lost: 344,440 bytes in 14,350 blocks
>> ...
>>
>> Summary after the fix:
>>
>> LEAK SUMMARY:
>> definitely lost: 344,376 bytes in 14,349 blocks
>> ...
>>
>> Signed-off-by: Ulf Magnusson <[email protected]>
>> ---
>> scripts/kconfig/zconf.y | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/scripts/kconfig/zconf.y b/scripts/kconfig/zconf.y
>> index 468ab03..3c9f436 100644
>> --- a/scripts/kconfig/zconf.y
>> +++ b/scripts/kconfig/zconf.y
>> @@ -435,6 +435,9 @@ help_start: T_HELP T_EOL
>>
>> help: help_start T_HELPTEXT
>> {
>> + if (current_entry->help)
>> + /* Weird menu node with two help strings */
>> + free(current_entry->help);
>> current_entry->help = $2;
>> };
>
>
> I applied this, but maybe is it better to warning message?
> zconfprint("warning: ...") or zconf_error()?
>
>
> If you send v2, I will replace this.

Yeah, that's a good idea.

New version at https://marc.info/?l=linux-kernel&m=151573969713437&w=2.
The warning seems more important than the leak, so I rephrased it a
bit.

Cheers,
Ulf