Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1465063 > unrolled thread

[PATCH 0/5] block-cciss: Fine-tuning for two function implementations

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 0/5] block-cciss: Fine-tuning for two function implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-18 12:00 +0200
    [PATCH 1/5] block-cciss: Use memdup_user() rather than duplicating  its implementation SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-18 12:00 +0200
    [PATCH 2/5] block-cciss: Less function calls in cciss_bigpassthru()  after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-18 12:00 +0200
    [PATCH 3/5] block-cciss: Delete unnecessary initialisations in  cciss_bigpassthru() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-18 12:10 +0200
    [PATCH 4/5] block-cciss: Move an assignment for the variable  "sg_used" in cciss_bigpassthru() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-18 12:10 +0200
    [PATCH 5/5] block-cciss: Replace three kzalloc() calls by kcalloc() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-18 12:10 +0200

#1465063 — [PATCH 0/5] block-cciss: Fine-tuning for two function implementations

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 18 Aug 2016 11:40:04 +0200

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

Markus Elfring (5):
  Use memdup_user()
  Less function calls after error detection
  Delete unnecessary initialisations
  Move an assignment for the variable "sg_used"
  Replace three kzalloc() calls by kcalloc()

 drivers/block/cciss.c | 66 ++++++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 35 deletions(-)

-- 
2.9.3

[toc] | [next] | [standalone]

#1465066 — [PATCH 1/5] block-cciss: Use memdup_user() rather than duplicating its implementation

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 17 Aug 2016 22:10:29 +0200

* Reuse existing functionality from memdup_user() instead of keeping
  duplicate source code.

  This issue was detected by using the Coccinelle software.

* Return directly if this copy operation failed.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/block/cciss.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index db9d6bb..e044342 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -1587,15 +1587,9 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user *argp)
 		return -EINVAL;
 	if (!capable(CAP_SYS_RAWIO))
 		return -EPERM;
-	ioc = kmalloc(sizeof(*ioc), GFP_KERNEL);
-	if (!ioc) {
-		status = -ENOMEM;
-		goto cleanup1;
-	}
-	if (copy_from_user(ioc, argp, sizeof(*ioc))) {
-		status = -EFAULT;
-		goto cleanup1;
-	}
+	ioc = memdup_user(argp, sizeof(*ioc));
+	if (IS_ERR(ioc))
+		return PTR_ERR(ioc);
 	if ((ioc->buf_size < 1) &&
 	    (ioc->Request.Type.Direction != XFER_NONE)) {
 		status = -EINVAL;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1465068 — [PATCH 2/5] block-cciss: Less function calls in cciss_bigpassthru() after error detection

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 17 Aug 2016 22:39:31 +0200

The kfree() function was called in a few cases by the
cciss_bigpassthru() function during error handling
even if a passed variable contained a null pointer.

Adjust jump targets according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/block/cciss.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index e044342..43ac632 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -1593,26 +1593,26 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user *argp)
 	if ((ioc->buf_size < 1) &&
 	    (ioc->Request.Type.Direction != XFER_NONE)) {
 		status = -EINVAL;
-		goto cleanup1;
+		goto free_ioc;
 	}
 	/* Check kmalloc limits  using all SGs */
 	if (ioc->malloc_size > MAX_KMALLOC_SIZE) {
 		status = -EINVAL;
-		goto cleanup1;
+		goto free_ioc;
 	}
 	if (ioc->buf_size > ioc->malloc_size * MAXSGENTRIES) {
 		status = -EINVAL;
-		goto cleanup1;
-	}
-	buff = kzalloc(MAXSGENTRIES * sizeof(char *), GFP_KERNEL);
-	if (!buff) {
-		status = -ENOMEM;
-		goto cleanup1;
+		goto free_ioc;
 	}
 	buff_size = kmalloc(MAXSGENTRIES * sizeof(int), GFP_KERNEL);
 	if (!buff_size) {
 		status = -ENOMEM;
-		goto cleanup1;
+		goto free_ioc;
+	}
+	buff = kzalloc(MAXSGENTRIES * sizeof(char *), GFP_KERNEL);
+	if (!buff) {
+		status = -ENOMEM;
+		goto free_size;
 	}
 	left = ioc->buf_size;
 	data_ptr = ioc->buf;
@@ -1622,12 +1622,12 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user *argp)
 		buff[sg_used] = kmalloc(sz, GFP_KERNEL);
 		if (buff[sg_used] == NULL) {
 			status = -ENOMEM;
-			goto cleanup1;
+			goto free_buffer;
 		}
 		if (ioc->Request.Type.Direction == XFER_WRITE) {
 			if (copy_from_user(buff[sg_used], data_ptr, sz)) {
 				status = -EFAULT;
-				goto cleanup1;
+				goto free_buffer;
 			}
 		} else {
 			memset(buff[sg_used], 0, sz);
@@ -1639,7 +1639,7 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user *argp)
 	c = cmd_special_alloc(h);
 	if (!c) {
 		status = -ENOMEM;
-		goto cleanup1;
+		goto free_buffer;
 	}
 	c->cmd_type = CMD_IOCTL_PEND;
 	c->Header.ReplyQueue = 0;
@@ -1674,7 +1674,7 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user *argp)
 	if (copy_to_user(argp, ioc, sizeof(*ioc))) {
 		cmd_special_free(h, c);
 		status = -EFAULT;
-		goto cleanup1;
+		goto free_buffer;
 	}
 	if (ioc->Request.Type.Direction == XFER_READ) {
 		/* Copy the data out of the buffer we created */
@@ -1683,20 +1683,20 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user *argp)
 			if (copy_to_user(ptr, buff[i], buff_size[i])) {
 				cmd_special_free(h, c);
 				status = -EFAULT;
-				goto cleanup1;
+				goto free_buffer;
 			}
 			ptr += buff_size[i];
 		}
 	}
 	cmd_special_free(h, c);
 	status = 0;
-cleanup1:
-	if (buff) {
-		for (i = 0; i < sg_used; i++)
-			kfree(buff[i]);
-		kfree(buff);
-	}
+free_buffer:
+	for (i = 0; i < sg_used; i++)
+		kfree(buff[i]);
+	kfree(buff);
+free_size:
 	kfree(buff_size);
+free_ioc:
 	kfree(ioc);
 	return status;
 }
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1465076 — [PATCH 3/5] block-cciss: Delete unnecessary initialisations in cciss_bigpassthru()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 17 Aug 2016 22:55:51 +0200

Three local variables will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/block/cciss.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 43ac632..10e1b0a 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -1572,11 +1572,11 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user *argp)
 {
 	BIG_IOCTL_Command_struct *ioc;
 	CommandList_struct *c;
-	unsigned char **buff = NULL;
-	int *buff_size = NULL;
+	unsigned char **buff;
+	int *buff_size;
 	u64bit temp64;
 	BYTE sg_used = 0;
-	int status = 0;
+	int status;
 	int i;
 	DECLARE_COMPLETION_ONSTACK(wait);
 	__u32 left;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1465078 — [PATCH 4/5] block-cciss: Move an assignment for the variable "sg_used" in cciss_bigpassthru()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 17 Aug 2016 23:04:46 +0200

Move the assignment for the local variable "sg_used" behind the source code
for some memory allocations by this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/block/cciss.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 10e1b0a..b08bfb7 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -1575,7 +1575,7 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user *argp)
 	unsigned char **buff;
 	int *buff_size;
 	u64bit temp64;
-	BYTE sg_used = 0;
+	BYTE sg_used;
 	int status;
 	int i;
 	DECLARE_COMPLETION_ONSTACK(wait);
@@ -1616,6 +1616,7 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user *argp)
 	}
 	left = ioc->buf_size;
 	data_ptr = ioc->buf;
+	sg_used = 0;
 	while (left) {
 		sz = (left > ioc->malloc_size) ? ioc->malloc_size : left;
 		buff_size[sg_used] = sz;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1465080 — [PATCH 5/5] block-cciss: Replace three kzalloc() calls by kcalloc()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 18 Aug 2016 11:26:18 +0200

* The script "checkpatch.pl" can point information out like the following.

  WARNING: Prefer kcalloc over kzalloc with multiply

  Thus fix the affected source code places.

* Replace the specification of data structures by pointer dereferences
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

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

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index b08bfb7..3502a3a 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -1604,12 +1604,12 @@ static int cciss_bigpassthru(ctlr_info_t *h, void __user *argp)
 		status = -EINVAL;
 		goto free_ioc;
 	}
-	buff_size = kmalloc(MAXSGENTRIES * sizeof(int), GFP_KERNEL);
+	buff_size = kcalloc(MAXSGENTRIES, sizeof(*buff_size), GFP_KERNEL);
 	if (!buff_size) {
 		status = -ENOMEM;
 		goto free_ioc;
 	}
-	buff = kzalloc(MAXSGENTRIES * sizeof(char *), GFP_KERNEL);
+	buff = kcalloc(MAXSGENTRIES, sizeof(*buff), GFP_KERNEL);
 	if (!buff) {
 		status = -ENOMEM;
 		goto free_size;
@@ -4838,8 +4838,9 @@ static int cciss_allocate_scatterlists(ctlr_info_t *h)
 	int i;
 
 	/* zero it, so that on free we need not know how many were alloc'ed */
-	h->scatter_list = kzalloc(h->max_commands *
-				sizeof(struct scatterlist *), GFP_KERNEL);
+	h->scatter_list = kcalloc(h->max_commands,
+				  sizeof(*h->scatter_list),
+				  GFP_KERNEL);
 	if (!h->scatter_list)
 		return -ENOMEM;
 
-- 
2.9.3

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web