[PATCH 0/4] char-SNSC: Adjustments for six function implementations

[PATCH 0/4] char-SNSC: Adjustments for six function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 16 Oct 2017 17:00:02 +0200

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

Markus Elfring (4):
  Adjust one function call together with a variable assignment
  Delete three error messages for a failed memory allocation
  Improve a size determination in two functions
  Delete unnecessary braces in five functions

 drivers/char/snsc.c | 50 ++++++++++++++++++--------------------------------
 1 file changed, 18 insertions(+), 32 deletions(-)

-- 
2.14.2

[PATCH 1/4] char/snsc: Adjust one function call together with a variable assignment

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 16 Oct 2017 16:15:21 +0200

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

ERROR: do not use assignment in if condition

Thus fix the affected source code place.

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

diff --git a/drivers/char/snsc.c b/drivers/char/snsc.c
index 6aa32679fd58..f551a625686a 100644
--- a/drivers/char/snsc.c
+++ b/drivers/char/snsc.c
@@ -419,7 +419,8 @@ scdrv_init(void)
 
 			/* initialize sysctl device data fields */
 			scd->scd_nasid = cnodeid_to_nasid(cnode);
-			if (!(salbuf = kmalloc(SCDRV_BUFSZ, GFP_KERNEL))) {
+			salbuf = kmalloc(SCDRV_BUFSZ, GFP_KERNEL);
+			if (!salbuf) {
 				printk("%s: failed to allocate driver buffer"
 				       "(%s%s)\n", __func__,
 				       SYSCTL_BASENAME, devname);
-- 
2.14.2

[PATCH 2/4] char/snsc: Delete three error messages for a failed memory allocation

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 16 Oct 2017 16:24:09 +0200

Omit extra messages for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/char/snsc.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/char/snsc.c b/drivers/char/snsc.c
index f551a625686a..eefc306033ab 100644
--- a/drivers/char/snsc.c
+++ b/drivers/char/snsc.c
@@ -80,11 +80,8 @@ scdrv_open(struct inode *inode, struct file *file)
 
 	/* allocate memory for subchannel data */
 	sd = kzalloc(sizeof (struct subch_data_s), GFP_KERNEL);
-	if (sd = NULL) {
-		printk("%s: couldn't allocate subchannel data\n",
-		       __func__);
+	if (!sd)
 		return -ENOMEM;
-	}
 
 	/* initialize subch_data_s fields */
 	sd->sd_nasid = scd->scd_nasid;
@@ -410,20 +407,13 @@ scdrv_init(void)
 			/* allocate sysctl device data */
 			scd = kzalloc(sizeof (struct sysctl_data_s),
 				      GFP_KERNEL);
-			if (!scd) {
-				printk("%s: failed to allocate device info"
-				       "for %s/%s\n", __func__,
-				       SYSCTL_BASENAME, devname);
+			if (!scd)
 				continue;
-			}
 
 			/* initialize sysctl device data fields */
 			scd->scd_nasid = cnodeid_to_nasid(cnode);
 			salbuf = kmalloc(SCDRV_BUFSZ, GFP_KERNEL);
 			if (!salbuf) {
-				printk("%s: failed to allocate driver buffer"
-				       "(%s%s)\n", __func__,
-				       SYSCTL_BASENAME, devname);
 				kfree(scd);
 				continue;
 			}
-- 
2.14.2

[PATCH 3/4] char/snsc: Improve a size determination in two functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 16 Oct 2017 16:26:09 +0200

Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

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

diff --git a/drivers/char/snsc.c b/drivers/char/snsc.c
index eefc306033ab..bec3bd966b1f 100644
--- a/drivers/char/snsc.c
+++ b/drivers/char/snsc.c
@@ -79,7 +79,7 @@ scdrv_open(struct inode *inode, struct file *file)
 	scd = container_of(inode->i_cdev, struct sysctl_data_s, scd_cdev);
 
 	/* allocate memory for subchannel data */
-	sd = kzalloc(sizeof (struct subch_data_s), GFP_KERNEL);
+	sd = kzalloc(sizeof(*sd), GFP_KERNEL);
 	if (!sd)
 		return -ENOMEM;
 
@@ -405,8 +405,7 @@ scdrv_init(void)
 				geo_slab(geoid));
 
 			/* allocate sysctl device data */
-			scd = kzalloc(sizeof (struct sysctl_data_s),
-				      GFP_KERNEL);
+			scd = kzalloc(sizeof(*scd), GFP_KERNEL);
 			if (!scd)
 				continue;
 
-- 
2.14.2

[PATCH 4/4] char/snsc: Delete unnecessary braces in five functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 16 Oct 2017 16:48:24 +0200

Do not use curly brackets at some source code places
where a single statement should be sufficient.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/char/snsc.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/char/snsc.c b/drivers/char/snsc.c
index bec3bd966b1f..aea0ee083511 100644
--- a/drivers/char/snsc.c
+++ b/drivers/char/snsc.c
@@ -47,9 +47,9 @@ scdrv_interrupt(int irq, void *subch_data)
 	status = ia64_sn_irtr_intr(sd->sd_nasid, sd->sd_subch);
 
 	if (status > 0) {
-		if (status & SAL_IROUTER_INTR_RECV) {
+		if (status & SAL_IROUTER_INTR_RECV)
 			wake_up(&sd->sd_rq);
-		}
+
 		if (status & SAL_IROUTER_INTR_XMIT) {
 			ia64_sn_irtr_intr_disable
 			    (sd->sd_nasid, sd->sd_subch,
@@ -165,14 +165,13 @@ scdrv_read(struct file *file, char __user *buf, size_t count, loff_t *f_pos)
 		/* somebody else has it now;
 		 * if we're non-blocking, then exit...
 		 */
-		if (file->f_flags & O_NONBLOCK) {
+		if (file->f_flags & O_NONBLOCK)
 			return -EAGAIN;
-		}
+
 		/* ...or if we want to block, then do so here */
-		if (down_interruptible(&sd->sd_rbs)) {
+		if (down_interruptible(&sd->sd_rbs))
 			/* something went wrong with wait */
 			return -ERESTARTSYS;
-		}
 	}
 
 	/* anything to read? */
@@ -257,14 +256,13 @@ scdrv_write(struct file *file, const char __user *buf,
 		/* somebody else has it now;
 		 * if we're non-blocking, then exit...
 		 */
-		if (file->f_flags & O_NONBLOCK) {
+		if (file->f_flags & O_NONBLOCK)
 			return -EAGAIN;
-		}
+
 		/* ...or if we want to block, then do so here */
-		if (down_interruptible(&sd->sd_wbs)) {
+		if (down_interruptible(&sd->sd_wbs))
 			/* something went wrong with wait */
 			return -ERESTARTSYS;
-		}
 	}
 
 	count = min((int) count, CHUNKSIZE);
@@ -336,12 +334,11 @@ scdrv_poll(struct file *file, struct poll_table_struct *wait)
 	spin_unlock_irqrestore(&sd->sd_rlock, flags);
 
 	if (status > 0) {
-		if (status & SAL_IROUTER_INTR_RECV) {
+		if (status & SAL_IROUTER_INTR_RECV)
 			mask |= POLLIN | POLLRDNORM;
-		}
-		if (status & SAL_IROUTER_INTR_XMIT) {
+
+		if (status & SAL_IROUTER_INTR_XMIT)
 			mask |= POLLOUT | POLLWRNORM;
-		}
 	}
 
 	return mask;
@@ -450,9 +447,8 @@ scdrv_init(void)
                         /* on the console nasid, prepare to receive
                          * system controller environmental events
                          */
-                        if(scd->scd_nasid = event_nasid) {
+			if (scd->scd_nasid = event_nasid)
                                 scdrv_event_init(scd);
-                        }
 	}
 	return 0;
 }
-- 
2.14.2

Re: [PATCH 4/4] char/snsc: Delete unnecessary braces in five functions

[Dan Carpenter]

On Mon, Oct 16, 2017 at 05:07:29PM +0200, SF Markus Elfring wrote:
>  		/* ...or if we want to block, then do so here */
> -		if (down_interruptible(&sd->sd_wbs)) {
> +		if (down_interruptible(&sd->sd_wbs))
>  			/* something went wrong with wait */
>  			return -ERESTARTSYS;
> -		}
>  	}

Multi-line indents get parenthesis.

regards,
dan carpenter

Re: [PATCH 4/4] char/snsc: Delete unnecessary braces in five functions

[SF Markus Elfring]

>>  		/* ...or if we want to block, then do so here */
>> -		if (down_interruptible(&sd->sd_wbs)) {
>> +		if (down_interruptible(&sd->sd_wbs))
>>  			/* something went wrong with wait */
>>  			return -ERESTARTSYS;
>> -		}
>>  	}
> 
> Multi-line indents get parenthesis.

How do you think about to add such an expectation to the section
“3) Placing Braces and Spaces” in the document “coding-style.rst”?

Regards,
Markus