Re: [PATCH] mm/ksm: Fix -Wsometimes-uninitialized from clang-21 in advisor_mode_show()

[David Hildenbrand <david@redhat.com>]

On 15.07.25 23:49, Andrew Morton wrote:
> On Tue, 15 Jul 2025 12:56:16 -0700 Nathan Chancellor <nathan@kernel.org> wrote:
> 
>> After a recent change in clang to expose uninitialized warnings from
>> const variables [1], there is a warning from the if statement in
>> advisor_mode_show().
> 
> I'll change this to "a false positive warning".
> 
>>    mm/ksm.c:3687:11: error: variable 'output' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
>>     3687 |         else if (ksm_advisor == KSM_ADVISOR_SCAN_TIME)
>>          |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>    mm/ksm.c:3690:33: note: uninitialized use occurs here
>>     3690 |         return sysfs_emit(buf, "%s\n", output);
>>          |                                        ^~~~~~
>>
>> Rewrite the if statement to implicitly make KSM_ADVISOR_NONE the else
>> branch so that it is obvious to the compiler that ksm_advisor can only
>> be KSM_ADVISOR_NONE or KSM_ADVISOR_SCAN_TIME due to the assignments in
>> advisor_mode_store().
>>
>> --- a/mm/ksm.c
>> +++ b/mm/ksm.c
>> @@ -3682,10 +3682,10 @@ static ssize_t advisor_mode_show(struct kobject *kobj,
>>   {
>>   	const char *output;
>>   
>> -	if (ksm_advisor == KSM_ADVISOR_NONE)
>> -		output = "[none] scan-time";
>> -	else if (ksm_advisor == KSM_ADVISOR_SCAN_TIME)
>> +	if (ksm_advisor == KSM_ADVISOR_SCAN_TIME)
>>   		output = "none [scan-time]";
>> +	else
>> +		output = "[none] scan-time";
>>   
>>   	return sysfs_emit(buf, "%s\n", output);
>>   }
> 
> Ho hum OK, but the code did deteriorate a bit.
> 
> static ssize_t advisor_mode_show(struct kobject *kobj,
> 				 struct kobj_attribute *attr, char *buf)
> {
> 	const char *output;
> 
> 	if (ksm_advisor == KSM_ADVISOR_SCAN_TIME)
> 		output = "none [scan-time]";
> 	else
> 		output = "[none] scan-time";
> 
> 	return sysfs_emit(buf, "%s\n", output);
> }
> 
> Inconsistent with the other code which looks at this enum.  Previously
> the code explicitly recognized that there are only two modes and that
> became implicit.
> 
> Oh well, no big deal and we don't want clang builds erroring out like
> this.

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb