[PATCH 0/3] InfiniBand-SRP: Fine-tuning for some function implementations

[PATCH 0/3] InfiniBand-SRP: Fine-tuning for some function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 8 Apr 2017 14:04:04 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Use kmalloc_array() in srp_alloc_req_data()
  Enclose 14 expressions for the sizeof operator by parentheses
  Delete unwanted spaces behind usages of the sizeof operator

 drivers/infiniband/ulp/srp/ib_srp.c | 59 +++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 29 deletions(-)

-- 
2.12.2

[PATCH 1/3] IB/srp: Use kmalloc_array() in srp_alloc_req_data()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 8 Apr 2017 11:11:44 +0200

Multiplications for the size determination of memory allocations
indicated that array data structures should be processed.
Thus use the corresponding function "kmalloc_array".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index cee46266f434..cf8cb7206983 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -866,16 +866,17 @@ static int srp_alloc_req_data(struct srp_rdma_ch *ch)
 
 	for (i = 0; i < target->req_ring_size; ++i) {
 		req = &ch->req_ring[i];
-		mr_list = kmalloc(target->mr_per_cmd * sizeof(void *),
-				  GFP_KERNEL);
+		mr_list = kmalloc_array(target->mr_per_cmd, sizeof(void *),
+					GFP_KERNEL);
 		if (!mr_list)
 			goto out;
 		if (srp_dev->use_fast_reg) {
 			req->fr_list = mr_list;
 		} else {
 			req->fmr_list = mr_list;
-			req->map_page = kmalloc(srp_dev->max_pages_per_mr *
-						sizeof(void *), GFP_KERNEL);
+			req->map_page = kmalloc_array(srp_dev->max_pages_per_mr,
+						      sizeof(void *),
+						      GFP_KERNEL);
 			if (!req->map_page)
 				goto out;
 		}
-- 
2.12.2

[PATCH 2/3] IB/srp: Enclose 14 expressions for the sizeof operator by parentheses

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 8 Apr 2017 13:49:53 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script "checkpatch.pl" pointed information out like the following.

WARNING: sizeof … should be sizeof(…)

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index cf8cb7206983..bc934c5f1e36 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -213,8 +213,8 @@ static int srp_target_is_topspin(struct srp_target_port *target)
 	static const u8 cisco_oui[3]   = { 0x00, 0x1b, 0x0d };
 
 	return topspin_workarounds &&
-		(!memcmp(&target->ioc_guid, topspin_oui, sizeof topspin_oui) ||
-		 !memcmp(&target->ioc_guid, cisco_oui, sizeof cisco_oui));
+		(!memcmp(&target->ioc_guid, topspin_oui, sizeof(topspin_oui)) ||
+		 !memcmp(&target->ioc_guid, cisco_oui, sizeof(cisco_oui)));
 }
 
 static struct srp_iu *srp_alloc_iu(struct srp_host *host, size_t size,
@@ -223,7 +223,7 @@ static struct srp_iu *srp_alloc_iu(struct srp_host *host, size_t size,
 {
 	struct srp_iu *iu;
 
-	iu = kmalloc(sizeof *iu, gfp_mask);
+	iu = kmalloc(sizeof(*iu), gfp_mask);
 	if (!iu)
 		goto out;
 
@@ -272,7 +272,7 @@ static int srp_init_qp(struct srp_target_port *target,
 	struct ib_qp_attr *attr;
 	int ret;
 
-	attr = kmalloc(sizeof *attr, GFP_KERNEL);
+	attr = kmalloc(sizeof(*attr), GFP_KERNEL);
 	if (!attr)
 		return -ENOMEM;
 
@@ -488,7 +488,7 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch)
 	const int m = 1 + dev->use_fast_reg * target->mr_per_cmd * 2;
 	int ret;
 
-	init_attr = kzalloc(sizeof *init_attr, GFP_KERNEL);
+	init_attr = kzalloc(sizeof(*init_attr), GFP_KERNEL);
 	if (!init_attr)
 		return -ENOMEM;
 
@@ -703,7 +703,7 @@ static int srp_send_req(struct srp_rdma_ch *ch, bool multich)
 	} *req = NULL;
 	int status;
 
-	req = kzalloc(sizeof *req, GFP_KERNEL);
+	req = kzalloc(sizeof(*req), GFP_KERNEL);
 	if (!req)
 		return -ENOMEM;
 
@@ -713,7 +713,7 @@ static int srp_send_req(struct srp_rdma_ch *ch, bool multich)
 	req->param.qp_num		      = ch->qp->qp_num;
 	req->param.qp_type		      = ch->qp->qp_type;
 	req->param.private_data 	      = &req->priv;
-	req->param.private_data_len 	      = sizeof req->priv;
+	req->param.private_data_len	      = sizeof(req->priv);
 	req->param.flow_control 	      = 1;
 
 	get_random_bytes(&req->param.starting_psn, 4);
@@ -2164,7 +2164,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)
 	scmnd->host_scribble = (void *) req;
 
 	cmd = iu->buf;
-	memset(cmd, 0, sizeof *cmd);
+	memset(cmd, 0, sizeof(*cmd));
 
 	cmd->opcode = SRP_CMD;
 	int_to_scsilun(scmnd->device->lun, &cmd->lun);
@@ -2347,7 +2347,7 @@ static void srp_cm_rep_handler(struct ib_cm_id *cm_id,
 	}
 
 	ret = -ENOMEM;
-	qp_attr = kmalloc(sizeof *qp_attr, GFP_KERNEL);
+	qp_attr = kmalloc(sizeof(*qp_attr), GFP_KERNEL);
 	if (!qp_attr)
 		goto error;
 
@@ -2575,10 +2575,10 @@ static int srp_send_tsk_mgmt(struct srp_rdma_ch *ch, u64 req_tag, u64 lun,
 		return -1;
 	}
 
-	ib_dma_sync_single_for_cpu(dev, iu->dma, sizeof *tsk_mgmt,
+	ib_dma_sync_single_for_cpu(dev, iu->dma, sizeof(*tsk_mgmt),
 				   DMA_TO_DEVICE);
 	tsk_mgmt = iu->buf;
-	memset(tsk_mgmt, 0, sizeof *tsk_mgmt);
+	memset(tsk_mgmt, 0, sizeof(*tsk_mgmt));
 
 	tsk_mgmt->opcode 	= SRP_TSK_MGMT;
 	int_to_scsilun(lun, &tsk_mgmt->lun);
@@ -2592,7 +2592,7 @@ static int srp_send_tsk_mgmt(struct srp_rdma_ch *ch, u64 req_tag, u64 lun,
 
 	init_completion(&ch->tsk_mgmt_done);
 
-	ib_dma_sync_single_for_device(dev, iu->dma, sizeof *tsk_mgmt,
+	ib_dma_sync_single_for_device(dev, iu->dma, sizeof(*tsk_mgmt),
 				      DMA_TO_DEVICE);
 	if (srp_post_send(ch, iu, sizeof(*tsk_mgmt))) {
 		srp_put_tx_iu(ch, iu, SRP_IU_TSK_MGMT);
@@ -3308,7 +3308,7 @@ static ssize_t srp_create_target(struct device *dev,
 	target_host->max_channel = 0;
 	target_host->max_id      = 1;
 	target_host->max_lun     = -1LL;
-	target_host->max_cmd_len = sizeof ((struct srp_cmd *) (void *) 0L)->cdb;
+	target_host->max_cmd_len = sizeof(((struct srp_cmd *)(void *)0L)->cdb);
 
 	target = host_to_target(target_host);
 
@@ -3535,7 +3535,7 @@ static struct srp_host *srp_add_port(struct srp_device *device, u8 port)
 {
 	struct srp_host *host;
 
-	host = kzalloc(sizeof *host, GFP_KERNEL);
+	host = kzalloc(sizeof(*host), GFP_KERNEL);
 	if (!host)
 		return NULL;
 
-- 
2.12.2

Re: [PATCH 2/3] IB/srp: Enclose 14 expressions for the sizeof operator by parentheses

[Joe Perches]

On Sat, 2017-04-08 at 14:12 +0200, SF Markus Elfring wrote:
[]
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
[]
> @@ -3308,7 +3308,7 @@ static ssize_t srp_create_target(struct device *dev,
>  	target_host->max_channel = 0;
>  	target_host->max_id      = 1;
>  	target_host->max_lun     = -1LL;
> -	target_host->max_cmd_len = sizeof ((struct srp_cmd *) (void *) 0L)->cdb;
> +	target_host->max_cmd_len = sizeof(((struct srp_cmd *)(void *)0L)->cdb);

Maybe better as:

	target_host->max_cmd_len = FIELD_SIZEOF(struct srp_cmd, cdb);

[PATCH 3/3] IB/srp: Delete unwanted spaces behind usages of the sizeof operator

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 8 Apr 2017 13:56:33 +0200

Replace the source code "sizeof (" by "sizeof("
according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index bc934c5f1e36..0229328e65e3 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1628,7 +1628,7 @@ static int srp_map_data(struct scsi_cmnd *scmnd, struct srp_rdma_ch *ch,
 	u8 fmt;
 
 	if (!scsi_sglist(scmnd) || scmnd->sc_data_direction = DMA_NONE)
-		return sizeof (struct srp_cmd);
+		return sizeof(struct srp_cmd);
 
 	if (scmnd->sc_data_direction != DMA_FROM_DEVICE &&
 	    scmnd->sc_data_direction != DMA_TO_DEVICE) {
@@ -1649,7 +1649,7 @@ static int srp_map_data(struct scsi_cmnd *scmnd, struct srp_rdma_ch *ch,
 		return -EIO;
 
 	fmt = SRP_DATA_DESC_DIRECT;
-	len = sizeof (struct srp_cmd) +	sizeof (struct srp_direct_buf);
+	len = sizeof(struct srp_cmd) + sizeof(struct srp_direct_buf);
 
 	if (count = 1 && (pd->flags & IB_PD_UNSAFE_GLOBAL_RKEY)) {
 		/*
@@ -1722,15 +1722,15 @@ static int srp_map_data(struct scsi_cmnd *scmnd, struct srp_rdma_ch *ch,
 	}
 
 	count = min(state.ndesc, target->cmd_sg_cnt);
-	table_len = state.ndesc * sizeof (struct srp_direct_buf);
+	table_len = state.ndesc * sizeof(struct srp_direct_buf);
 	idb_len = sizeof(struct srp_indirect_buf) + table_len;
 
 	fmt = SRP_DATA_DESC_INDIRECT;
-	len = sizeof(struct srp_cmd) + sizeof (struct srp_indirect_buf);
-	len += count * sizeof (struct srp_direct_buf);
+	len = sizeof(struct srp_cmd) + sizeof(struct srp_indirect_buf);
+	len += count * sizeof(struct srp_direct_buf);
 
 	memcpy(indirect_hdr->desc_list, req->indirect_desc,
-	       count * sizeof (struct srp_direct_buf));
+	       count * sizeof(struct srp_direct_buf));
 
 	if (!(pd->flags & IB_PD_UNSAFE_GLOBAL_RKEY)) {
 		ret = srp_map_idb(ch, req, state.gen.next, state.gen.end,
@@ -3300,7 +3300,7 @@ static ssize_t srp_create_target(struct device *dev,
 	bool multich = false;
 
 	target_host = scsi_host_alloc(&srp_template,
-				      sizeof (struct srp_target_port));
+				      sizeof(struct srp_target_port));
 	if (!target_host)
 		return -ENOMEM;
 
@@ -3383,10 +3383,10 @@ static ssize_t srp_create_target(struct device *dev,
 	target->mr_pool_size = target->scsi_host->can_queue * mr_per_cmd;
 	target->mr_per_cmd = mr_per_cmd;
 	target->indirect_size = target->sg_tablesize *
-				sizeof (struct srp_direct_buf);
-	target->max_iu_len = sizeof (struct srp_cmd) +
-			     sizeof (struct srp_indirect_buf) +
-			     target->cmd_sg_cnt * sizeof (struct srp_direct_buf);
+				sizeof(struct srp_direct_buf);
+	target->max_iu_len = sizeof(struct srp_cmd) +
+			     sizeof(struct srp_indirect_buf) +
+			     target->cmd_sg_cnt * sizeof(struct srp_direct_buf);
 
 	INIT_WORK(&target->tl_err_work, srp_tl_err_work);
 	INIT_WORK(&target->remove_work, srp_remove_work);
-- 
2.12.2