linux-kernel - Re: [PATCH v2] char: mwave: smapi: Fix signedness of SmapiOK variable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d5e06499-781a-4bc0-a43b-8ef2aa89930b@gmail.com>
Date: Thu, 17 Apr 2025 15:19:58 +0530
From: Purva Yeshi <purvayeshi550@...il.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: arnd@...db.de, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] char: mwave: smapi: Fix signedness of SmapiOK variable

On 17/04/25 15:08, Greg KH wrote:
> On Thu, Apr 17, 2025 at 02:40:18PM +0530, Purva Yeshi wrote:
>> Smatch warning:
>> drivers/char/mwave/smapi.c:69 smapi_request() warn:
>> assigning (-5) to unsigned variable 'usSmapiOK'
>>
>> Fix Smatch warning caused by assigning -EIO to an unsigned short.
>>
>> Smatch detected a warning due to assigning -EIO (a negative value) to an
>> unsigned short variable, causing a type mismatch and potential issues.
>>
>> In v1, the type was changed to short, which resolved the warning, but
>> retained the misleading "us" prefix in the variable name.
>>
>> In v2, update the type to s16 and rename the variable to SmapiOK,
>> removing the "us" (unsigned short) prefix as per Greg KH suggestion.
>>
>> This change ensures type correctness, avoids confusion, and improves
>> overall code readability.
>>
>> Signed-off-by: Purva Yeshi <purvayeshi550@...il.com>
>> ---
>> V1 - https://lore.kernel.org/all/20250409211929.213360-1-purvayeshi550@gmail.com/
>> V2 - Use s16 type and rename variable to remove misleading "us" prefix.
>>
>>   drivers/char/mwave/smapi.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/char/mwave/smapi.c b/drivers/char/mwave/smapi.c
>> index f8d79d393b69..65bc7e1ea6cf 100644
>> --- a/drivers/char/mwave/smapi.c
>> +++ b/drivers/char/mwave/smapi.c
>> @@ -66,7 +66,7 @@ static int smapi_request(unsigned short inBX, unsigned short inCX,
>>   	unsigned short myoutDX = 5, *pmyoutDX = &myoutDX;
>>   	unsigned short myoutDI = 6, *pmyoutDI = &myoutDI;
>>   	unsigned short myoutSI = 7, *pmyoutSI = &myoutSI;
>> -	unsigned short usSmapiOK = -EIO, *pusSmapiOK = &usSmapiOK;
>> +	s16 SmapiOK = -EIO, *pSmapiOK = &SmapiOK;
> 
> Do you think that "SmapiOK" is a valid kernel variable name?  Doesn't
> look ok to me, what does checkpatch.pl say?  :)
> 
> thanks,
> 
> greg k-h

Hi Greg,

Thank you for the feedback.

I ran checkpatch.pl on the patch, and it reports 0 errors and 0 
warnings, so the variable name "SmapiOK" is valid in this context.

Regarding the "usSmapiOK" variable, it was used only within the smapi.c 
file. Therefore, I have renamed it consistently throughout the file to 
avoid the misleading "us" prefix.

Let me know if you have any further suggestions.

Best regards,
Purva