Re: [PATCH 1/7] x86: don't leak sub-architecture accumulated strings

[Jan Beulich <jbeulich@suse.com>]

On 01.07.2022 00:54, H.J. Lu wrote:
> On Thu, Jun 30, 2022 at 5:53 AM Jan Beulich <jbeulich@suse.com> wrote:
>> --- a/gas/config/tc-i386.c
>> +++ b/gas/config/tc-i386.c
>> @@ -2871,6 +2871,7 @@ set_cpu_arch (int dummy ATTRIBUTE_UNUSED
>>                   check_cpu_arch_compatible (string, cpu_arch[j].flags);
>>
>>                   cpu_arch_name = cpu_arch[j].name;
>> +                 xfree (cpu_sub_arch_name);
>>                   cpu_sub_arch_name = NULL;
>>                   cpu_arch_flags = cpu_arch[j].flags;
>>                   if (flag_code == CODE_64BIT)
>> @@ -13406,6 +13407,7 @@ md_parse_option (int c, const char *arg)
>>                     continue;
>>
>>                   cpu_arch_name = cpu_arch[j].name;
>> +                 xfree (cpu_sub_arch_name);
>>                   cpu_sub_arch_name = NULL;
>>                   cpu_arch_flags = cpu_arch[j].flags;
>>                   cpu_arch_isa = cpu_arch[j].type;
>> @@ -14086,6 +14088,7 @@ i386_target_format (void)
>>         {
>>           static const i386_cpu_flags iamcu_flags = CPU_IAMCU_FLAGS;
>>           cpu_arch_name = "iamcu";
>> +         xfree (cpu_sub_arch_name);
>>           cpu_sub_arch_name = NULL;
>>           cpu_arch_flags = iamcu_flags;
>>           cpu_arch_isa = PROCESSOR_IAMCU;
>>
> 
> Can we just use free?

In principle yes, but I view using free() as inconsistent when the
allocation used xmalloc() or alike. These should pair, even if right
now xfree() simply aliases to free() - this could change down the
road, and then cause unnecessary code churn just to update all such
instances.

Jan