Re: [PATCH] selftests/mm: mlock2-tests: fix fclose(NULL) in is_vmflag_set()

[Hongfu Li <hongfu.li@linux.dev>]

在 11/12/26 03:49, Liam R. Howlett 写道:
> On 26/11/04 04:49PM, Hongfu Li wrote:
>> From: Hongfu Li <lihongfu@kylinos.cn>
>>
>> is_vmflag_set() jumps to the shared 'out' label when
>> seek_to_smaps_entry() fails. smaps is NULL on that path, and the
>> code at the 'out' label calls fclose(smaps) unconditionally.
>> fclose(NULL) is undefined behaviour, so the test crashes before
>> it can report a test result.
>>
>> Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
> The sending address should match the sign off.

Sure, I'll send a follow-up from the same address.

>> ---
>>   tools/testing/selftests/mm/mlock2-tests.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/mm/mlock2-tests.c b/tools/testing/selftests/mm/mlock2-tests.c
>> index 144b550813a6..85e6cd24515a 100644
>> --- a/tools/testing/selftests/mm/mlock2-tests.c
>> +++ b/tools/testing/selftests/mm/mlock2-tests.c
>> @@ -66,7 +66,7 @@ static bool is_vmflag_set(unsigned long addr, const char *vmflag)
>>   	smaps = seek_to_smaps_entry(addr);
>>   	if (!smaps) {
>>   		ksft_print_msg("Unable to parse /proc/self/smaps\n");
>> -		goto out;
>> +		return false;
>>   	}
> This looks better.
>
> The next function is also messy with the goto usage/ordering of
> unrolling and should probably be changed in the same way.

Thanks for the review.

Agreed, get_value_for_name() has the same pattern. I'll handle it in
the same way in v2.

>>   
>>   	while (getline(&line, &size, smaps) > 0) {
>>
>> ---
>> base-commit: 74d73ffcffe03804f97e9fb82301dadf75671282
>> change-id: 20261104-for-mlock2-a67f86dad498
>>
>> Best regards,
>> --
>> Hongfu Li <lihongfu@kylinos.cn>
>>
-- 
Best regards,
Hongfu