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

From: Greg KH
Date: Thu Apr 17 2025 - 05:38:43 EST


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@xxxxxxxxx>
> ---
> V1 - https://lore.kernel.org/all/20250409211929.213360-1-purvayeshi550@xxxxxxxxx/
> 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