| Message ID | 20180124124301.10884-1-yuval.shaia@oracle.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
On Wed, Jan 24, 2018 at 02:43:01PM +0200, Yuval Shaia wrote: > To be compatible with other modules/drivers, change return code checks > from "if (rc != 0)" to "if (rc)". > > Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com> > --- > v0 -> v1: > * Add some other fixes (srp,iser and srpt) > v1 -> v2: > * Accept Leon's comment and change commit's title. > --- > drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c | 8 ++++---- > drivers/infiniband/ulp/iser/iser_initiator.c | 2 +- > drivers/infiniband/ulp/srp/ib_srp.c | 2 +- > drivers/infiniband/ulp/srpt/ib_srpt.c | 4 ++-- > 4 files changed, 8 insertions(+), 8 deletions(-) > Thanks, Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
On Wed, 2018-01-24 at 14:43 +0200, Yuval Shaia wrote: > To be compatible with other modules/drivers, change return code checks > from "if (rc != 0)" to "if (rc)". Sorry but I think this is a completely pointless change. All the code that is touched by this patch works fine, conforms to all appropriate coding style guides and is easy to read. So why change it? Bart.
On Wed, 2018-01-24 at 15:06 +0000, Bart Van Assche wrote: > On Wed, 2018-01-24 at 14:43 +0200, Yuval Shaia wrote: > > To be compatible with other modules/drivers, change return code checks > > from "if (rc != 0)" to "if (rc)". > > Sorry but I think this is a completely pointless change. All the code that is > touched by this patch works fine, conforms to all appropriate coding style > guides and is easy to read. So why change it? > > Bart. Hi Yuval, Since about 1/3 of the LOC in this is in srp/srpt, and the maintainer objects and thinks the patch is unnecessary, I'm inclined to drop this unless you want to make an argument for why it should go on in.
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c index d650a9fcde24..ce289a55a9dc 100644 --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c @@ -837,16 +837,16 @@ static int pvrdma_pci_probe(struct pci_dev *pdev, } /* Enable 64-Bit DMA */ - if (pci_set_dma_mask(pdev, DMA_BIT_MASK(64)) == 0) { + if (!pci_set_dma_mask(pdev, DMA_BIT_MASK(64))) { ret = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64)); - if (ret != 0) { + if (ret) { dev_err(&pdev->dev, "pci_set_consistent_dma_mask failed\n"); goto err_free_resource; } } else { ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32)); - if (ret != 0) { + if (ret) { dev_err(&pdev->dev, "pci_set_dma_mask failed\n"); goto err_free_resource; @@ -1018,7 +1018,7 @@ static int pvrdma_pci_probe(struct pci_dev *pdev, /* Check if device was successfully activated */ ret = pvrdma_read_reg(dev, PVRDMA_REG_ERR); - if (ret != 0) { + if (ret) { dev_err(&pdev->dev, "failed to activate device\n"); ret = -EFAULT; goto err_disable_intr; diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c index 2a07692007bd..09045a432f89 100644 --- a/drivers/infiniband/ulp/iser/iser_initiator.c +++ b/drivers/infiniband/ulp/iser/iser_initiator.c @@ -128,7 +128,7 @@ iser_prepare_write_cmd(struct iscsi_task *task, err = iser_reg_rdma_mem(iser_task, ISER_DIR_OUT, buf_out->data_len == imm_sz); - if (err != 0) { + if (err) { iser_err("Failed to register write cmd RDMA mem\n"); return err; } diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 62d88212c1b0..8d9676b9fbc9 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -2109,7 +2109,7 @@ static void srp_recv_done(struct ib_cq *cq, struct ib_wc *wc) DMA_FROM_DEVICE); res = srp_post_recv(ch, iu); - if (res != 0) + if (res) shost_printk(KERN_ERR, target->scsi_host, PFX "Recv failed with error code %d\n", res); } diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c index d78f60dcc2ba..659e6d62fb97 100644 --- a/drivers/infiniband/ulp/srpt/ib_srpt.c +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c @@ -1441,7 +1441,7 @@ static void srpt_handle_cmd(struct srpt_rdma_ch *ch, scsilun_to_int(&srp_cmd->lun), data_len, TCM_SIMPLE_TAG, dir, TARGET_SCF_ACK_KREF, sg, sg_cnt, NULL, 0, NULL, 0); - if (rc != 0) { + if (rc) { pr_debug("target_submit_cmd() returned %d for tag %#llx\n", rc, srp_cmd->tag); goto release_ioctx; @@ -1508,7 +1508,7 @@ static void srpt_handle_tsk_mgmt(struct srpt_rdma_ch *ch, scsilun_to_int(&srp_tsk->lun), srp_tsk, tcm_tmr, GFP_KERNEL, srp_tsk->task_tag, TARGET_SCF_ACK_KREF); - if (rc != 0) { + if (rc) { send_ioctx->cmd.se_tmr_req->response = TMR_FUNCTION_REJECTED; goto fail; }
To be compatible with other modules/drivers, change return code checks from "if (rc != 0)" to "if (rc)". Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com> --- v0 -> v1: * Add some other fixes (srp,iser and srpt) v1 -> v2: * Accept Leon's comment and change commit's title. --- drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c | 8 ++++---- drivers/infiniband/ulp/iser/iser_initiator.c | 2 +- drivers/infiniband/ulp/srp/ib_srp.c | 2 +- drivers/infiniband/ulp/srpt/ib_srpt.c | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-)