linux-kernel - Re: [PATCH] platform/x86: intel-vbtn: Fix code style issues

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: <fdb9c21f-aada-498a-92ec-bc48aceeb76e@kernel.org>
Date: Fri, 20 Jun 2025 12:00:03 +0200
From: Hans de Goede <hansg@...nel.org>
To: Xiang Shen <turyshen@...il.com>, acelan.kao@...onical.com,
 ilpo.jarvinen@...ux.intel.com, platform-driver-x86@...r.kernel.org
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH] platform/x86: intel-vbtn: Fix code style issues

Hi,

On 20-Jun-25 2:38 AM, Xiang Shen wrote:
> Fix checkpatch code style errors:
> 
> ERROR: do not use assignment in if condition
> +	if ((ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event))) {
> 
> ERROR: do not use assignment in if condition
> +	} else if ((ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event))) {
> 
> Signed-off-by: Xiang Shen <turyshen@...il.com>

Thank you for your patch, but this change really does not make
the code more readable.

The contrary the suggested changes are making the code harder
to read, so NACK.

Note checkpatch is just a tool, sometimes there are good reasons
to deviate from the style checks done by checkpatch.

Next time when submitting a patch to fix checkpatch issues please
take a look at the resulting code after the patch and only submit
the patch upstream if it actually is an improvement.

Regards,

Hans



> ---
>  drivers/platform/x86/intel/vbtn.c | 38 +++++++++++++++++--------------
>  1 file changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/vbtn.c b/drivers/platform/x86/intel/vbtn.c
> index 232cd12e3c9f..bcc97b06844e 100644
> --- a/drivers/platform/x86/intel/vbtn.c
> +++ b/drivers/platform/x86/intel/vbtn.c
> @@ -160,30 +160,34 @@ static void notify_handler(acpi_handle handle, u32 event, void *context)
>  
>  	guard(mutex)(&priv->mutex);
>  
> -	if ((ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event))) {
> +	ke = sparse_keymap_entry_from_scancode(priv->buttons_dev, event);
> +	if (ke) {
>  		if (!priv->has_buttons) {
>  			dev_warn(&device->dev, "Warning: received 0x%02x button event on a device without buttons, please report this.\n",
>  				 event);
>  			return;
>  		}
>  		input_dev = priv->buttons_dev;
> -	} else if ((ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event))) {
> -		if (!priv->has_switches) {
> -			/* See dual_accel_detect.h for more info */
> -			if (priv->dual_accel)
> -				return;
> -
> -			dev_info(&device->dev, "Registering Intel Virtual Switches input-dev after receiving a switch event\n");
> -			ret = input_register_device(priv->switches_dev);
> -			if (ret)
> -				return;
> -
> -			priv->has_switches = true;
> -		}
> -		input_dev = priv->switches_dev;
>  	} else {
> -		dev_dbg(&device->dev, "unknown event index 0x%x\n", event);
> -		return;
> +		ke = sparse_keymap_entry_from_scancode(priv->switches_dev, event);
> +		if (ke) {
> +			if (!priv->has_switches) {
> +				/* See dual_accel_detect.h for more info */
> +				if (priv->dual_accel)
> +					return;
> +
> +				dev_info(&device->dev, "Registering Intel Virtual Switches input-dev after receiving a switch event\n");
> +				ret = input_register_device(priv->switches_dev);
> +				if (ret)
> +					return;
> +
> +				priv->has_switches = true;
> +			}
> +			input_dev = priv->switches_dev;
> +		} else {
> +			dev_dbg(&device->dev, "unknown event index 0x%x\n", event);
> +			return;
> +		}
>  	}
>  
>  	if (priv->wakeup_mode) {