[PATCH 1/2] nvmet-auth: remove unnecessary break after goto

[PATCH 1/2] nvmet-auth: remove unnecessary break after goto

[Chaitanya Kulkarni]

Remove dead break after goto.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
 drivers/nvme/target/fabrics-cmd-auth.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
index 7970a7640e58..6c9a1ce6068d 100644
--- a/drivers/nvme/target/fabrics-cmd-auth.c
+++ b/drivers/nvme/target/fabrics-cmd-auth.c
@@ -295,13 +295,11 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
 			status = 0;
 		}
 		goto done_kfree;
-		break;
 	case NVME_AUTH_DHCHAP_MESSAGE_SUCCESS2:
 		req->sq->authenticated = true;
 		pr_debug("%s: ctrl %d qid %d ctrl authenticated\n",
 			 __func__, ctrl->cntlid, req->sq->qid);
 		goto done_kfree;
-		break;
 	case NVME_AUTH_DHCHAP_MESSAGE_FAILURE2:
 		status = nvmet_auth_failure2(d);
 		if (status) {
@@ -312,7 +310,6 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
 			status = 0;
 		}
 		goto done_kfree;
-		break;
 	default:
 		req->sq->dhchap_status =
 			NVME_AUTH_DHCHAP_FAILURE_INCORRECT_MESSAGE;
@@ -320,7 +317,6 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
 			NVME_AUTH_DHCHAP_MESSAGE_FAILURE2;
 		req->sq->authenticated = false;
 		goto done_kfree;
-		break;
 	}
 done_failure1:
 	req->sq->dhchap_status = NVME_AUTH_DHCHAP_FAILURE_INCORRECT_MESSAGE;
-- 
2.40.0

[PATCH 2/2] nvme-fcloop: no need to return from void function

[Chaitanya Kulkarni]

Remove return at the end of void function.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
 drivers/nvme/target/fcloop.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
index e940a7d37a9d..1ab3d900f2bf 100644
--- a/drivers/nvme/target/fcloop.c
+++ b/drivers/nvme/target/fcloop.c
@@ -645,8 +645,6 @@ fcloop_fcp_recv_work(struct work_struct *work)
 	}
 	if (ret)
 		fcloop_call_host_done(fcpreq, tfcp_req, ret);
-
-	return;
 }
 
 static void
-- 
2.40.0

Re: [PATCH 1/2] nvmet-auth: remove unnecessary break after goto

[Christoph Hellwig]

On Fri, May 19, 2023 at 02:40:51AM -0700, Chaitanya Kulkarni wrote:
> Remove dead break after goto.

As a trivial cleanup this looks fine, but I really hate the structure
of this code.

Can't we have local variables that we set to the dhchap_status and
dhchap_step, and then after the done label just conditionally assign
them if dhchap_status is non-zero?  This should clean up a lot of
mess, especially we'd be done to very few assignments of the step,
and we could avoid jumping out of the switch entirely.

Re: [PATCH 2/2] nvme-fcloop: no need to return from void function

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/2] nvmet-auth: remove unnecessary break after goto

[Chaitanya Kulkarni]

On 5/19/2023 10:01 PM, Christoph Hellwig wrote:
> On Fri, May 19, 2023 at 02:40:51AM -0700, Chaitanya Kulkarni wrote:
>> Remove dead break after goto.
> 
> As a trivial cleanup this looks fine, but I really hate the structure
> of this code.
> 
> Can't we have local variables that we set to the dhchap_status and
> dhchap_step, and then after the done label just conditionally assign
> them if dhchap_status is non-zero?  This should clean up a lot of
> mess, especially we'd be done to very few assignments of the step,
> and we could avoid jumping out of the switch entirely.

similar cleanup is also needed in nvmet_execute_auth_receive(),
in general nvmet_execute_auth_receive() and nvmet_execute_auth_send()
has a lot of common code that can use cleanup, I've a series on the
top of this one ..

but prior to that we need these two patches to make both functions
close enough so I can send a final cleanup on the top of this ..

-ck

Re: [PATCH 2/2] nvme-fcloop: no need to return from void function

[Max Gurtovoy]

Hi CK,

On 19/05/2023 12:40, Chaitanya Kulkarni wrote:
> Remove return at the end of void function.
> 
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
>   drivers/nvme/target/fcloop.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
> index e940a7d37a9d..1ab3d900f2bf 100644
> --- a/drivers/nvme/target/fcloop.c
> +++ b/drivers/nvme/target/fcloop.c
> @@ -645,8 +645,6 @@ fcloop_fcp_recv_work(struct work_struct *work)
>   	}
>   	if (ret)
>   		fcloop_call_host_done(fcpreq, tfcp_req, ret);
> -
> -	return;
>   }
>   
>   static void

Looks good,

Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>


BTW, are patches 1/2 and 2/2 related to each other ?
Can you split between them please if not related.

Re: [PATCH 1/2] nvmet-auth: remove unnecessary break after goto

[Keith Busch]

Thanks, applied both for nvme-6.5. Looking forward to your follow up
clean up that depended on this :)

Re: [PATCH 1/2] nvmet-auth: remove unnecessary break after goto

[Nitesh Shetty]

[-- Attachment #1: Type: text/plain, Size: 1450 bytes --]

On 23/05/19 02:40AM, Chaitanya Kulkarni wrote:
>Remove dead break after goto.
>
>Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
>---
> drivers/nvme/target/fabrics-cmd-auth.c | 4 ----
> 1 file changed, 4 deletions(-)
>
>diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
>index 7970a7640e58..6c9a1ce6068d 100644
>--- a/drivers/nvme/target/fabrics-cmd-auth.c
>+++ b/drivers/nvme/target/fabrics-cmd-auth.c
>@@ -295,13 +295,11 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
> 			status = 0;
> 		}
> 		goto done_kfree;
>-		break;
> 	case NVME_AUTH_DHCHAP_MESSAGE_SUCCESS2:
> 		req->sq->authenticated = true;
> 		pr_debug("%s: ctrl %d qid %d ctrl authenticated\n",
> 			 __func__, ctrl->cntlid, req->sq->qid);
> 		goto done_kfree;
>-		break;
> 	case NVME_AUTH_DHCHAP_MESSAGE_FAILURE2:
> 		status = nvmet_auth_failure2(d);
> 		if (status) {
>@@ -312,7 +310,6 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
> 			status = 0;
> 		}
> 		goto done_kfree;
>-		break;
> 	default:
> 		req->sq->dhchap_status =
> 			NVME_AUTH_DHCHAP_FAILURE_INCORRECT_MESSAGE;
>@@ -320,7 +317,6 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
> 			NVME_AUTH_DHCHAP_MESSAGE_FAILURE2;
> 		req->sq->authenticated = false;
> 		goto done_kfree;
>-		break;
> 	}
> done_failure1:
> 	req->sq->dhchap_status = NVME_AUTH_DHCHAP_FAILURE_INCORRECT_MESSAGE;
>-- 
>2.40.0
>
>

Reviewed-by: Nitesh Shetty <nj.shetty@samsung.com>

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] nvme-fcloop: no need to return from void function

[Nitesh Shetty]

[-- Attachment #1: Type: text/plain, Size: 647 bytes --]

On 23/05/19 02:40AM, Chaitanya Kulkarni wrote:
>Remove return at the end of void function.
>
>Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
>---
> drivers/nvme/target/fcloop.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
>index e940a7d37a9d..1ab3d900f2bf 100644
>--- a/drivers/nvme/target/fcloop.c
>+++ b/drivers/nvme/target/fcloop.c
>@@ -645,8 +645,6 @@ fcloop_fcp_recv_work(struct work_struct *work)
> 	}
> 	if (ret)
> 		fcloop_call_host_done(fcpreq, tfcp_req, ret);
>-
>-	return;
> }
>
> static void
>-- 
>2.40.0
>
>
Reviewed-by: Nitesh Shetty <nj.shetty@samsung.com>

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]