[PATCH 1/1] lib: utils: Simplify SET_ISA_EXT_MAP()

Heinrich Schuchardt heinrich.schuchardt at canonical.com
Wed Sep 27 08:38:25 PDT 2023


On 9/27/23 17:23, Xiang W wrote:
> 在 2023-09-27星期三的 16:21 +0200,Heinrich Schuchardt写道:
>> The define is hard to read.
>>
>> Addresses-Coverity-ID: 1568359 Unexpected control flow
>> Fixes: d72f5f17478d ("lib: utils: Add detection of Smepmp from ISA string in FDT")
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
>> ---
>>   lib/utils/fdt/fdt_helper.c | 8 +++-----
>>   1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
>> index c97f09d..6f23f6f 100644
>> --- a/lib/utils/fdt/fdt_helper.c
>> +++ b/lib/utils/fdt/fdt_helper.c
>> @@ -370,12 +370,10 @@ static int fdt_parse_isa_one_hart(const char *isa, unsigned long *extensions)
>>   			continue;
>>   
>>   #define SET_ISA_EXT_MAP(name, bit)				\
>> -		do {						\
>> -			if (!strcmp(mstr, name)) {		\
>> +		{						\
>> +			if (!strcmp(mstr, name))		\
>>   				__set_bit(bit, extensions);	\
>> -				continue;			\
> Only remove 'do ... while' and 'continue' can be keep . In this way, after finding
> the extension, the subsequent judgment can be skip.

I understand why you want to have a continue here. Makes sense to me 
once you add more invocations of the macro.

Just to be clear this is not what the incoming code did.

With your suggestion we should rename the macro to something like 
SET_ISA_EXT_AND_CONTINUE() to make the behavior obvious.

Best regards

Heinrich

> 
> Regards,
> Xiang W
>> -			}					\
>> -		} while (false)					\
>> +		}						\
>>   
>>   		SET_ISA_EXT_MAP("smepmp", SBI_HART_EXT_SMEPMP);
>>   #undef SET_ISA_EXT_MAP
>> -- 
>> 2.40.1
>>
>>
> 




More information about the opensbi mailing list