diff mbox series

[linux-next] RDMA: simplify if-if to if-else

Message ID 20220328130900.8539-1-guozhengkui@vivo.com (mailing list archive)
State Superseded
Headers show
Series [linux-next] RDMA: simplify if-if to if-else | expand

Commit Message

Guo Zhengkui March 28, 2022, 1:08 p.m. UTC
`if (!ret)` can be replaced with `else` for simplification.

Signed-off-by: Guo Zhengkui <guozhengkui@vivo.com>
---
 drivers/infiniband/hw/irdma/puda.c | 4 ++--
 drivers/infiniband/hw/mlx4/mcg.c   | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

Comments

Leon Romanovsky March 30, 2022, 11:02 a.m. UTC | #1
On Mon, Mar 28, 2022 at 09:08:59PM +0800, Guo Zhengkui wrote:
> `if (!ret)` can be replaced with `else` for simplification.
> 
> Signed-off-by: Guo Zhengkui <guozhengkui@vivo.com>
> ---
>  drivers/infiniband/hw/irdma/puda.c | 4 ++--
>  drivers/infiniband/hw/mlx4/mcg.c   | 3 +--
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Haakon Bugge March 30, 2022, 11:06 a.m. UTC | #2
> On 30 Mar 2022, at 13:02, Leon Romanovsky <leon@kernel.org> wrote:
> 
> On Mon, Mar 28, 2022 at 09:08:59PM +0800, Guo Zhengkui wrote:
>> `if (!ret)` can be replaced with `else` for simplification.
>> 
>> Signed-off-by: Guo Zhengkui <guozhengkui@vivo.com>
>> ---
>> drivers/infiniband/hw/irdma/puda.c | 4 ++--
>> drivers/infiniband/hw/mlx4/mcg.c   | 3 +--
>> 2 files changed, 3 insertions(+), 4 deletions(-)
>> 
> 
> Thanks,
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

Fix the unbalanced curly brackets at the same time?


Thxs, Håkon
Leon Romanovsky March 30, 2022, 11:32 a.m. UTC | #3
On Wed, Mar 30, 2022 at 11:06:03AM +0000, Haakon Bugge wrote:
> 
> 
> > On 30 Mar 2022, at 13:02, Leon Romanovsky <leon@kernel.org> wrote:
> > 
> > On Mon, Mar 28, 2022 at 09:08:59PM +0800, Guo Zhengkui wrote:
> >> `if (!ret)` can be replaced with `else` for simplification.
> >> 
> >> Signed-off-by: Guo Zhengkui <guozhengkui@vivo.com>
> >> ---
> >> drivers/infiniband/hw/irdma/puda.c | 4 ++--
> >> drivers/infiniband/hw/mlx4/mcg.c   | 3 +--
> >> 2 files changed, 3 insertions(+), 4 deletions(-)
> >> 
> > 
> > Thanks,
> > Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
> 
> Fix the unbalanced curly brackets at the same time?

I think that it is ok to have if () ... else { ... } code.

There is one place that needs an indentation fix, in mlx4, but it is
faster to fix when applying the patch instead of asking to resubmit.

thanks

> 
> 
> Thxs, Håkon
> 
>
Haakon Bugge March 30, 2022, 12:26 p.m. UTC | #4
> On 30 Mar 2022, at 13:32, Leon Romanovsky <leon@kernel.org> wrote:
> 
> On Wed, Mar 30, 2022 at 11:06:03AM +0000, Haakon Bugge wrote:
>> 
>> 
>>> On 30 Mar 2022, at 13:02, Leon Romanovsky <leon@kernel.org> wrote:
>>> 
>>> On Mon, Mar 28, 2022 at 09:08:59PM +0800, Guo Zhengkui wrote:
>>>> `if (!ret)` can be replaced with `else` for simplification.
>>>> 
>>>> Signed-off-by: Guo Zhengkui <guozhengkui@vivo.com>
>>>> ---
>>>> drivers/infiniband/hw/irdma/puda.c | 4 ++--
>>>> drivers/infiniband/hw/mlx4/mcg.c   | 3 +--
>>>> 2 files changed, 3 insertions(+), 4 deletions(-)
>>>> 
>>> 
>>> Thanks,
>>> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
>> 
>> Fix the unbalanced curly brackets at the same time?
> 
> I think that it is ok to have if () ... else { ... } code.


Hmm, doesn't the kernel coding style say:

"Do not unnecessarily use braces where a single statement will do."

[snip]

"This does not apply if only one branch of a conditional statement is a single statement; in the latter case use braces in both branches"


Thxs, Håkon


> 
> There is one place that needs an indentation fix, in mlx4, but it is
> faster to fix when applying the patch instead of asking to resubmit.
> 
> thanks
> 
>> 
>> 
>> Thxs, Håkon
Leon Romanovsky March 30, 2022, 12:56 p.m. UTC | #5
On Wed, Mar 30, 2022 at 12:26:51PM +0000, Haakon Bugge wrote:
> 
> 
> > On 30 Mar 2022, at 13:32, Leon Romanovsky <leon@kernel.org> wrote:
> > 
> > On Wed, Mar 30, 2022 at 11:06:03AM +0000, Haakon Bugge wrote:
> >> 
> >> 
> >>> On 30 Mar 2022, at 13:02, Leon Romanovsky <leon@kernel.org> wrote:
> >>> 
> >>> On Mon, Mar 28, 2022 at 09:08:59PM +0800, Guo Zhengkui wrote:
> >>>> `if (!ret)` can be replaced with `else` for simplification.
> >>>> 
> >>>> Signed-off-by: Guo Zhengkui <guozhengkui@vivo.com>
> >>>> ---
> >>>> drivers/infiniband/hw/irdma/puda.c | 4 ++--
> >>>> drivers/infiniband/hw/mlx4/mcg.c   | 3 +--
> >>>> 2 files changed, 3 insertions(+), 4 deletions(-)
> >>>> 
> >>> 
> >>> Thanks,
> >>> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
> >> 
> >> Fix the unbalanced curly brackets at the same time?
> > 
> > I think that it is ok to have if () ... else { ... } code.
> 
> 
> Hmm, doesn't the kernel coding style say:
> 
> "Do not unnecessarily use braces where a single statement will do."
> 
> [snip]
> 
> "This does not apply if only one branch of a conditional statement is a single statement; in the latter case use braces in both branches"

ok, if it is written in documentation, let's follow it.

Thanks for pointing that out.

> 
> 
> Thxs, Håkon
> 
> 
> > 
> > There is one place that needs an indentation fix, in mlx4, but it is
> > faster to fix when applying the patch instead of asking to resubmit.
> > 
> > thanks
> > 
> >> 
> >> 
> >> Thxs, Håkon
>
Guo Zhengkui March 31, 2022, 3:03 a.m. UTC | #6
On 2022/3/30 20:56, Leon Romanovsky wrote:
> On Wed, Mar 30, 2022 at 12:26:51PM +0000, Haakon Bugge wrote:
>>
>>
>>> On 30 Mar 2022, at 13:32, Leon Romanovsky <leon@kernel.org> wrote:
>>>
>>> On Wed, Mar 30, 2022 at 11:06:03AM +0000, Haakon Bugge wrote:
>>>>
>>>>
>>>>> On 30 Mar 2022, at 13:02, Leon Romanovsky <leon@kernel.org> wrote:
>>>>>
>>>>> On Mon, Mar 28, 2022 at 09:08:59PM +0800, Guo Zhengkui wrote:
>>>>>> `if (!ret)` can be replaced with `else` for simplification.
>>>>>>
>>>>>> Signed-off-by: Guo Zhengkui <guozhengkui@vivo.com>
>>>>>> ---
>>>>>> drivers/infiniband/hw/irdma/puda.c | 4 ++--
>>>>>> drivers/infiniband/hw/mlx4/mcg.c   | 3 +--
>>>>>> 2 files changed, 3 insertions(+), 4 deletions(-)
>>>>>>
>>>>>
>>>>> Thanks,
>>>>> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
>>>>
>>>> Fix the unbalanced curly brackets at the same time?
>>>
>>> I think that it is ok to have if () ... else { ... } code.
>>
>>
>> Hmm, doesn't the kernel coding style say:
>>
>> "Do not unnecessarily use braces where a single statement will do."
>>
>> [snip]
>>
>> "This does not apply if only one branch of a conditional statement is a single statement; in the latter case use braces in both branches"
> 
> ok, if it is written in documentation, let's follow it.
> 
> Thanks for pointing that out.

Should I resubmit the patch including unbalanced curly brackets fixing? 
If not, I can submit another patch to fix this problem.

> 
>>
>>
>> Thxs, Håkon
>>
>>
>>>
>>> There is one place that needs an indentation fix, in mlx4, but it is
>>> faster to fix when applying the patch instead of asking to resubmit.
>>>
>>> thanks
>>>
>>>>
>>>>
>>>> Thxs, Håkon
>>

Thanks,

Zhengkui
Leon Romanovsky March 31, 2022, 9:05 a.m. UTC | #7
On Thu, Mar 31, 2022 at 11:03:30AM +0800, Guo Zhengkui wrote:
> On 2022/3/30 20:56, Leon Romanovsky wrote:
> > On Wed, Mar 30, 2022 at 12:26:51PM +0000, Haakon Bugge wrote:
> > > 
> > > 
> > > > On 30 Mar 2022, at 13:32, Leon Romanovsky <leon@kernel.org> wrote:
> > > > 
> > > > On Wed, Mar 30, 2022 at 11:06:03AM +0000, Haakon Bugge wrote:
> > > > > 
> > > > > 
> > > > > > On 30 Mar 2022, at 13:02, Leon Romanovsky <leon@kernel.org> wrote:
> > > > > > 
> > > > > > On Mon, Mar 28, 2022 at 09:08:59PM +0800, Guo Zhengkui wrote:
> > > > > > > `if (!ret)` can be replaced with `else` for simplification.
> > > > > > > 
> > > > > > > Signed-off-by: Guo Zhengkui <guozhengkui@vivo.com>
> > > > > > > ---
> > > > > > > drivers/infiniband/hw/irdma/puda.c | 4 ++--
> > > > > > > drivers/infiniband/hw/mlx4/mcg.c   | 3 +--
> > > > > > > 2 files changed, 3 insertions(+), 4 deletions(-)
> > > > > > > 
> > > > > > 
> > > > > > Thanks,
> > > > > > Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
> > > > > 
> > > > > Fix the unbalanced curly brackets at the same time?
> > > > 
> > > > I think that it is ok to have if () ... else { ... } code.
> > > 
> > > 
> > > Hmm, doesn't the kernel coding style say:
> > > 
> > > "Do not unnecessarily use braces where a single statement will do."
> > > 
> > > [snip]
> > > 
> > > "This does not apply if only one branch of a conditional statement is a single statement; in the latter case use braces in both branches"
> > 
> > ok, if it is written in documentation, let's follow it.
> > 
> > Thanks for pointing that out.
> 
> Should I resubmit the patch including unbalanced curly brackets fixing? If
> not, I can submit another patch to fix this problem.

Your patch wasn't merged yet, so new version should be sent.

Thanks 
> 
> > 
> > > 
> > > 
> > > Thxs, Håkon
> > > 
> > > 
> > > > 
> > > > There is one place that needs an indentation fix, in mlx4, but it is
> > > > faster to fix when applying the patch instead of asking to resubmit.
> > > > 
> > > > thanks
> > > > 
> > > > > 
> > > > > 
> > > > > Thxs, Håkon
> > > 
> 
> Thanks,
> 
> Zhengkui
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/irdma/puda.c b/drivers/infiniband/hw/irdma/puda.c
index 397f3d070f90..ee91424eb94a 100644
--- a/drivers/infiniband/hw/irdma/puda.c
+++ b/drivers/infiniband/hw/irdma/puda.c
@@ -842,7 +842,7 @@  static void irdma_puda_free_qp(struct irdma_puda_rsrc *rsrc)
 		ibdev_dbg(to_ibdev(dev),
 			  "PUDA: error puda qp destroy wqe, status = %d\n",
 			  ret);
-	if (!ret) {
+	else {
 		ret = irdma_sc_poll_for_cqp_op_done(dev->cqp, IRDMA_CQP_OP_DESTROY_QP,
 						    &compl_info);
 		if (ret)
@@ -871,7 +871,7 @@  static void irdma_puda_free_cq(struct irdma_puda_rsrc *rsrc)
 	ret = irdma_sc_cq_destroy(&rsrc->cq, 0, true);
 	if (ret)
 		ibdev_dbg(to_ibdev(dev), "PUDA: error ieq cq destroy\n");
-	if (!ret) {
+	else {
 		ret = irdma_sc_poll_for_cqp_op_done(dev->cqp, IRDMA_CQP_OP_DESTROY_CQ,
 						    &compl_info);
 		if (ret)
diff --git a/drivers/infiniband/hw/mlx4/mcg.c b/drivers/infiniband/hw/mlx4/mcg.c
index 33f525b744f2..a8c8d432d0dc 100644
--- a/drivers/infiniband/hw/mlx4/mcg.c
+++ b/drivers/infiniband/hw/mlx4/mcg.c
@@ -304,9 +304,8 @@  static int send_leave_to_wire(struct mcast_group *group, u8 join_state)
 	ret = send_mad_to_wire(group->demux, (struct ib_mad *)&mad);
 	if (ret)
 		group->state = MCAST_IDLE;
-
 	/* set timeout handler */
-	if (!ret) {
+	else {
 		/* calls mlx4_ib_mcg_timeout_handler */
 		queue_delayed_work(group->demux->mcg_wq, &group->timeout_work,
 				msecs_to_jiffies(MAD_TIMEOUT_MS));