linux-kernel - Re: [PATCH] Update core.c

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: <7dcaa550-4c12-4c2e-9ae2-794c87048ea9@linuxfoundation.org>
Date: Mon, 30 Sep 2024 16:12:41 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: Okan Tumuklu <okantumukluu@...il.com>, shuah@...nel.org
Cc: linux-kernel@...r.kernel.org, Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH] Update core.c

On 9/30/24 16:06, Okan Tumuklu wrote:
> From: Okan Tümüklü <117488504+Okan-tumuklu@...rs.noreply.github.com>
> 
> 1:The control flow was simplified by using else if statements instead of goto structure.
> 
> 2:Error conditions are handled more clearly.
> 
> 3:The device_unlock call at the end of the function is guaranteed in all cases.

Write a paragraph - don't use bullet lists.

Please refer to submitting patches for details on how to
write shortlogs and change logs.

"Update core.c" with what? Write a better short log.

Why do you this 117488504+Okan-tumuklu@...rs.noreply.github.com
in the list? It will complain every time someone responds
to this thread. This is not how patches are sent. Refer to
documents in the kernel repo on how to send patches.

You are missing net maintainers and mailing lists.

Include all reviewers - run get_maintainers.pl

> ---
>   net/nfc/core.c | 28 ++++++++++------------------
>   1 file changed, 10 insertions(+), 18 deletions(-)
> 
> diff --git a/net/nfc/core.c b/net/nfc/core.c
> index e58dc6405054..4e8f01145c37 100644
> --- a/net/nfc/core.c
> +++ b/net/nfc/core.c
> @@ -40,27 +40,19 @@ int nfc_fw_download(struct nfc_dev *dev, const char *firmware_name)
>   
>   	if (dev->shutting_down) {
>   		rc = -ENODEV;
> -		goto error;
> -	}
> -
> -	if (dev->dev_up) {
> +	}else if (dev->dev_up) {
>   		rc = -EBUSY;
> -		goto error;
> -	}

Did you run checkpack script on this patch? There are a few
coding style errors.

> -
> -	if (!dev->ops->fw_download) {
> +	}else if (!dev->ops->fw_download) {
>   		rc = -EOPNOTSUPP;
> -		goto error;
> -	}
> -
> -	dev->fw_download_in_progress = true;
> -	rc = dev->ops->fw_download(dev, firmware_name);
> -	if (rc)
> -		dev->fw_download_in_progress = false;
> +	}else{
> +		dev->fw_download_in_progress = true;
> +		rc = dev->ops->fw_download(dev, firmware_name);
> +		if (rc)
> +			dev->fw_download_in_progress = false;
> +		}
>   
> -error:
> -	device_unlock(&dev->dev);
> -	return rc;
> +		device_unlock(&dev->dev);
> +		return rc;
>   }
>   
>   /**

thanks,
-- Shuah