Re: [PATCH v6 0/7] Add ARM Cortex-R52 CPU

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 6/12/22 12:43, Tobias Roehmel wrote:
> 
> On 06.12.22 11:39, Philippe Mathieu-Daudé wrote:
>> On 6/12/22 11:24, tobias.roehmel@rwth-aachen.de wrote:
>>> From: Tobias Röhmel <tobias.roehmel@rwth-aachen.de>
>>
>>> v6:
>>> patch 5:
>>> - I'm freeing the PRBAR/... strings explicitly now since
>>>    I don't know how to use autofree in this setup correctly.
>>>    Maybe {} around the part were the string is created/used,
>>>    such that it is dropped at }?
>>
>> The pointer is declared outside of a for() statement. Then
>> inside this statement you alloc/free twice, using the same
>> pointer. This is correct.  If you really want to use
>> g_autofree in such case, you'd need to declare within the
>> same statement, one pointer for each string:
>>
>>   for (i = 0; i < MIN(cpu->pmsav7_dregion, 32); ++i) {
>>       uint8_t crm = 0b1000 | extract32(i, 1, 3);
>>       uint8_t opc1 = extract32(i, 4, 1);
>>       uint8_t opc2 = extract32(i, 0, 1) << 2;
>>       g_autofree char *prbarn_str = g_strdup_printf("PRBAR%u", i);
>>       g_autofree char *prlarn_str = g_strdup_printf("PRLAR%u", i);
>>
>>       const ARMCPRegInfo tmp_prbarn_reginfo = {
>>           .name = prbarn_str, .type = ARM_CP_ALIAS | ARM_CP_NO_RAW,
>>           .cp = 15, .opc1 = opc1, .crn = 6, .crm = crm, .opc2 = opc2,
>>           .access = PL1_RW, .resetvalue = 0,
>>           .accessfn = access_tvm_trvm,
>>           .writefn = pmsav8r_regn_write, .readfn = pmsav8r_regn_read
>>       };
>>       define_one_arm_cp_reg(cpu, &tmp_prbarn_reginfo);
>>
>>       opc2 = extract32(i, 0, 1) << 2 | 0x1;
>>       const ARMCPRegInfo tmp_prlarn_reginfo = {
>>           .name = prlarn_str, .type = ARM_CP_ALIAS | ARM_CP_NO_RAW,
>>           .cp = 15, .opc1 = opc1, .crn = 6, .crm = crm, .opc2 = opc2,
>>           .access = PL1_RW, .resetvalue = 0,
>>           .accessfn = access_tvm_trvm,
>>           .writefn = pmsav8r_regn_write, .readfn = pmsav8r_regn_read
>>       };
>>       define_one_arm_cp_reg(cpu, &tmp_prlarn_reginfo);
>>   }
>>
>> (Note ARMCPRegInfo can be qualified const).
>>
>> Regards,
>>
>> Phil.
> 
> Thanks for the explanation! Would that be the preferred way?

What you posted is good enough IMO :) AFAIK there is no formal style
recommendation on using g_autofree.