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

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

[Andrew Morton]

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.

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

[David Hildenbrand]

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