[PATCH V2 0/7] drivers: isdn: act2000: fix checkpatch errors.

[PATCH V2 0/7] drivers: isdn: act2000: fix checkpatch errors.

[Bas Peters]

This patchset adresses many checkpatch errors found in the act2000 driver. 

Bas Peters (7):
  drivers: isdn: act2000: act2000_isa.c: Fix checkpatch errors
  drivers: isdn: act2000: capi.c: fix checkpatch errors
  drivers: isdn: act2000: remove assignments of variables in if
    conditions
  drivers: isdn: act2000: module.c: remove NULL-initialization of static
        variable.
  drivers: isdn: act2000: module.c: remove parenthesres around return   
     values.
  drivers: isdn: act2000: fix wrongly positioned brace.
  drivers: isdn: act2000: capi.c: add macro \ and fix brace

 drivers/isdn/act2000/act2000_isa.c | 11 +++++----
 drivers/isdn/act2000/capi.c        | 10 +++++---
 drivers/isdn/act2000/module.c      | 47 +++++++++++++++++++++++---------------
 3 files changed, 42 insertions(+), 26 deletions(-)

-- 
2.1.0

[PATCH 1/7] drivers: isdn: act2000: act2000_isa.c: Fix checkpatch errors

[Bas Peters]

This patch adresses various checkpatch errors:
	3 assignments in if conditions
	1 return value enclosed in parenthesis

Signed-off-by: Bas Peters <baspeters93@gmail.com>
---
 drivers/isdn/act2000/act2000_isa.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/isdn/act2000/act2000_isa.c b/drivers/isdn/act2000/act2000_isa.c
index b5fad29..048507e 100644
--- a/drivers/isdn/act2000/act2000_isa.c
+++ b/drivers/isdn/act2000/act2000_isa.c
@@ -31,7 +31,8 @@ act2000_isa_reset(unsigned short portbase)
 	int serial = 0;
 
 	found = 0;
-	if ((reg = inb(portbase + ISA_COR)) != 0xff) {
+	reg = inb(portbase + ISA_COR);
+	if (reg != 0xff) {
 		outb(reg | ISA_COR_RESET, portbase + ISA_COR);
 		mdelay(10);
 		outb(reg, portbase + ISA_COR);
@@ -303,7 +304,8 @@ act2000_isa_send(act2000_card *card)
 	while (1) {
 		spin_lock_irqsave(&card->lock, flags);
 		if (!(card->sbuf)) {
-			if ((card->sbuf = skb_dequeue(&card->sndq))) {
+			card->sbuf = skb_dequeue(&card->sndq);
+			if (card->sbuf) {
 				card->ack_msg = card->sbuf->data;
 				msg = (actcapi_msg *)card->sbuf->data;
 				if ((msg->hdr.cmd.cmd == 0x86) &&
@@ -378,7 +380,8 @@ act2000_isa_getid(act2000_card *card)
 		printk(KERN_WARNING "act2000: Wrong Firmware-ID!\n");
 		return -EPROTO;
 	}
-	if ((p = strchr(fid.revision, '\n')))
+	p = strchr(fid.revision, '\n');
+	if (p)
 		*p = '\0';
 	printk(KERN_INFO "act2000: Firmware-ID: %s\n", fid.revision);
 	if (card->flags & ACT2000_FLAGS_IVALID) {
@@ -439,5 +442,5 @@ act2000_isa_download(act2000_card *card, act2000_ddef __user *cb)
 	}
 	kfree(buf);
 	msleep_interruptible(500);
-	return (act2000_isa_getid(card));
+	return act2000_isa_getid(card);
 }
-- 
2.1.0

[PATCH 2/7] drivers: isdn: act2000: capi.c: fix checkpatch errors

[Bas Peters]

This patch fixes the following checkpatch errors:
	1. trailing statement
	1. assignment of variable in if condition
	1. incorrectly placed brace after function definition

Signed-off-by: Bas Peters <baspeters93@gmail.com>
---
 drivers/isdn/act2000/capi.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
index 3f66ca2..5d677e6 100644
--- a/drivers/isdn/act2000/capi.c
+++ b/drivers/isdn/act2000/capi.c
@@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
 			m->hdr.cmd.cmd = c;			\
 			m->hdr.cmd.subcmd = s;			\
 			m->hdr.msgnum = actcapi_nextsmsg(card); \
-		} else m = NULL;				\
+		} else
+			m = NULL;				\
 	}
 
 #define ACTCAPI_CHKSKB if (!skb) {					\
@@ -563,7 +564,8 @@ actcapi_data_b3_ind(act2000_card *card, struct sk_buff *skb) {
 	blocknr = msg->msg.data_b3_ind.blocknr;
 	skb_pull(skb, 19);
 	card->interface.rcvcallb_skb(card->myid, chan, skb);
-	if (!(skb = alloc_skb(11, GFP_ATOMIC))) {
+	skb = alloc_skb(11, GFP_ATOMIC);
+	if (!skb) {
 		printk(KERN_WARNING "actcapi: alloc_skb failed\n");
 		return 1;
 	}
@@ -990,7 +992,8 @@ actcapi_debug_dlpd(actcapi_dlpd *dlpd)
 }
 
 #ifdef DEBUG_DUMP_SKB
-static void dump_skb(struct sk_buff *skb) {
+static void dump_skb(struct sk_buff *skb)
+{
 	char tmp[80];
 	char *p = skb->data;
 	char *t = tmp;
-- 
2.1.0

[PATCH 3/7] drivers: isdn: act2000: remove assignments of variables in if conditions

[Bas Peters]

This patch removes all assignments of if conditions, which is not in
accordance with the CodingStyle.
---
 drivers/isdn/act2000/module.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/isdn/act2000/module.c b/drivers/isdn/act2000/module.c
index c3a1b06..352916a 100644
--- a/drivers/isdn/act2000/module.c
+++ b/drivers/isdn/act2000/module.c
@@ -289,7 +289,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
 			if (copy_from_user(tmp, arg,
 					   sizeof(tmp)))
 				return -EFAULT;
-			if ((ret = act2000_set_msn(card, tmp)))
+			ret = act2000_set_msn(card, tmp);
+			if (ret)
 				return ret;
 			if (card->flags & ACT2000_FLAGS_RUNNING)
 				return (actcapi_manufacturer_req_msn(card));
@@ -312,7 +313,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
 	case ISDN_CMD_DIAL:
 		if (!(card->flags & ACT2000_FLAGS_RUNNING))
 			return -ENODEV;
-		if (!(chan = find_channel(card, c->arg & 0x0f)))
+		chan = find_channel(card, c->arg & 0x0f);
+		if (!chan)
 			break;
 		spin_lock_irqsave(&card->lock, flags);
 		if (chan->fsm_state != ACT2000_STATE_NULL) {
@@ -341,7 +343,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
 	case ISDN_CMD_ACCEPTD:
 		if (!(card->flags & ACT2000_FLAGS_RUNNING))
 			return -ENODEV;
-		if (!(chan = find_channel(card, c->arg & 0x0f)))
+		chan = find_channel(card, c->arg & 0x0f);
+		if (!chan)
 			break;
 		if (chan->fsm_state == ACT2000_STATE_ICALL)
 			actcapi_select_b2_protocol_req(card, chan);
@@ -353,7 +356,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
 	case ISDN_CMD_HANGUP:
 		if (!(card->flags & ACT2000_FLAGS_RUNNING))
 			return -ENODEV;
-		if (!(chan = find_channel(card, c->arg & 0x0f)))
+		chan = find_channel(card, c->arg & 0x0f);
+		if (!chan)
 			break;
 		switch (chan->fsm_state) {
 		case ACT2000_STATE_ICALL:
@@ -368,7 +372,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
 	case ISDN_CMD_SETEAZ:
 		if (!(card->flags & ACT2000_FLAGS_RUNNING))
 			return -ENODEV;
-		if (!(chan = find_channel(card, c->arg & 0x0f)))
+		chan = find_channel(card, c->arg & 0x0f);
+		if (!chan)
 			break;
 		if (strlen(c->parm.num)) {
 			if (card->ptype == ISDN_PTYPE_EURO) {
@@ -388,7 +393,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
 	case ISDN_CMD_CLREAZ:
 		if (!(card->flags & ACT2000_FLAGS_RUNNING))
 			return -ENODEV;
-		if (!(chan = find_channel(card, c->arg & 0x0f)))
+		chan = find_channel(card, c->arg & 0x0f);
+		if (!chan)
 			break;
 		chan->eazmask = 0;
 		actcapi_listen_req(card);
@@ -396,7 +402,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
 	case ISDN_CMD_SETL2:
 		if (!(card->flags & ACT2000_FLAGS_RUNNING))
 			return -ENODEV;
-		if (!(chan = find_channel(card, c->arg & 0x0f)))
+		chan = find_channel(card, c->arg & 0x0f);
+		if (!chan)
 			break;
 		chan->l2prot = (c->arg >> 8);
 		return 0;
@@ -407,7 +414,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
 			printk(KERN_WARNING "L3 protocol unknown\n");
 			return -1;
 		}
-		if (!(chan = find_channel(card, c->arg & 0x0f)))
+		chan = find_channel(card, c->arg & 0x0f);
+		if (!chan)
 			break;
 		chan->l3prot = (c->arg >> 8);
 		return 0;
@@ -424,7 +432,8 @@ act2000_sendbuf(act2000_card *card, int channel, int ack, struct sk_buff *skb)
 	act2000_chan *chan;
 	actcapi_msg *msg;
 
-	if (!(chan = find_channel(card, channel)))
+	chan = find_channel(card, channel);
+	if (!chan)
 		return -1;
 	if (chan->fsm_state != ACT2000_STATE_ACTIVE)
 		return -1;
@@ -573,7 +582,8 @@ act2000_alloccard(int bus, int port, int irq, char *id)
 {
 	int i;
 	act2000_card *card;
-	if (!(card = kzalloc(sizeof(act2000_card), GFP_KERNEL))) {
+	card = kzalloc(sizeof(act2000_card), GFP_KERNEL);
+	if (!card) {
 		printk(KERN_WARNING
 		       "act2000: (%s) Could not allocate card-struct.\n", id);
 		return;
-- 
2.1.0

[PATCH 4/7] drivers: isdn: act2000: module.c: remove NULL-initialization of static variable.

[Bas Peters]

GCC takes care of this for us, thus it is not needed and theoretically
only hoggs memory, allbeit only a bit.
---
 drivers/isdn/act2000/module.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/isdn/act2000/module.c b/drivers/isdn/act2000/module.c
index 352916a..9359b36 100644
--- a/drivers/isdn/act2000/module.c
+++ b/drivers/isdn/act2000/module.c
@@ -28,7 +28,7 @@ static unsigned short act2000_isa_ports[] =
 static act2000_card *cards = (act2000_card *) NULL;
 
 /* Parameters to be set by insmod */
-static int   act_bus  =  0;
+static int   act_bus;
 static int   act_port = -1;  /* -1 = Autoprobe  */
 static int   act_irq  = -1;
 static char *act_id   = "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0";
-- 
2.1.0

[PATCH 5/7] drivers: isdn: act2000: module.c: remove parenthesres around return values.

[Bas Peters]

return is not a function, therefore parentheses are not needed.
---
 drivers/isdn/act2000/module.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/isdn/act2000/module.c b/drivers/isdn/act2000/module.c
index 9359b36..889ffcb 100644
--- a/drivers/isdn/act2000/module.c
+++ b/drivers/isdn/act2000/module.c
@@ -111,7 +111,7 @@ act2000_find_eaz(act2000_card *card, char eaz)
 
 	while (p) {
 		if (p->eaz == eaz)
-			return (p->msn);
+			return p->msn;
 		p = p->next;
 	}
 	return ("\0");
@@ -293,7 +293,7 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
 			if (ret)
 				return ret;
 			if (card->flags & ACT2000_FLAGS_RUNNING)
-				return (actcapi_manufacturer_req_msn(card));
+				return actcapi_manufacturer_req_msn(card);
 			return 0;
 		case ACT2000_IOCTL_ADDCARD:
 			if (copy_from_user(&cdef, arg,
@@ -520,7 +520,7 @@ if_command(isdn_ctrl *c)
 	act2000_card *card = act2000_findcard(c->driver);
 
 	if (card)
-		return (act2000_command(card, c));
+		return act2000_command(card, c);
 	printk(KERN_ERR
 	       "act2000: if_command %d called with invalid driverId %d!\n",
 	       c->command, c->driver);
@@ -535,7 +535,7 @@ if_writecmd(const u_char __user *buf, int len, int id, int channel)
 	if (card) {
 		if (!(card->flags & ACT2000_FLAGS_RUNNING))
 			return -ENODEV;
-		return (len);
+		return len;
 	}
 	printk(KERN_ERR
 	       "act2000: if_writecmd called with invalid driverId!\n");
@@ -550,7 +550,7 @@ if_readstatus(u_char __user *buf, int len, int id, int channel)
 	if (card) {
 		if (!(card->flags & ACT2000_FLAGS_RUNNING))
 			return -ENODEV;
-		return (act2000_readstatus(buf, len, card));
+		return act2000_readstatus(buf, len, card);
 	}
 	printk(KERN_ERR
 	       "act2000: if_readstatus called with invalid driverId!\n");
@@ -565,7 +565,7 @@ if_sendbuf(int id, int channel, int ack, struct sk_buff *skb)
 	if (card) {
 		if (!(card->flags & ACT2000_FLAGS_RUNNING))
 			return -ENODEV;
-		return (act2000_sendbuf(card, channel, ack, skb));
+		return act2000_sendbuf(card, channel, ack, skb);
 	}
 	printk(KERN_ERR
 	       "act2000: if_sendbuf called with invalid driverId!\n");
-- 
2.1.0

[PATCH 6/7] drivers: isdn: act2000: fix wrongly positioned brace.

[Bas Peters]

Trivial, but why not? :)

Signed-off-by: Bas Peters <baspeters93@gmail.com>
---
 drivers/isdn/act2000/module.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/isdn/act2000/module.c b/drivers/isdn/act2000/module.c
index 889ffcb..9ba98ce 100644
--- a/drivers/isdn/act2000/module.c
+++ b/drivers/isdn/act2000/module.c
@@ -19,8 +19,7 @@
 #include <linux/slab.h>
 #include <linux/init.h>
 
-static unsigned short act2000_isa_ports[] =
-{
+static unsigned short act2000_isa_ports[] = {
 	0x0200, 0x0240, 0x0280, 0x02c0, 0x0300, 0x0340, 0x0380,
 	0xcfe0, 0xcfa0, 0xcf60, 0xcf20, 0xcee0, 0xcea0, 0xce60,
 };
-- 
2.1.0

[PATCH 7/7] drivers: isdn: act2000: capi.c: add macro \ and fix brace

[Bas Peters]

This patch adds the \ that was accidentally deleted in patch 2. It also adds a brace after the else statement, which is required due to the fact that the if statement has braces.

---
 drivers/isdn/act2000/capi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
index 5d677e6..0043b3c 100644
--- a/drivers/isdn/act2000/capi.c
+++ b/drivers/isdn/act2000/capi.c
@@ -113,8 +113,9 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
 			m->hdr.cmd.cmd = c;			\
 			m->hdr.cmd.subcmd = s;			\
 			m->hdr.msgnum = actcapi_nextsmsg(card); \
-		} else
+		} else {					\
 			m = NULL;				\
+		}						\
 	}
 
 #define ACTCAPI_CHKSKB if (!skb) {					\
-- 
2.1.0

Re: [PATCH 2/7] drivers: isdn: act2000: capi.c: fix checkpatch errors

[Sergei Shtylyov]

On 02/07/2015 10:05 PM, Bas Peters wrote:

> This patch fixes the following checkpatch errors:
> 	1. trailing statement
> 	1. assignment of variable in if condition
> 	1. incorrectly placed brace after function definition

> Signed-off-by: Bas Peters <baspeters93@gmail.com>
> ---
>   drivers/isdn/act2000/capi.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)

> diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
> index 3f66ca2..5d677e6 100644
> --- a/drivers/isdn/act2000/capi.c
> +++ b/drivers/isdn/act2000/capi.c
> @@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
>   			m->hdr.cmd.cmd = c;			\
>   			m->hdr.cmd.subcmd = s;			\
>   			m->hdr.msgnum = actcapi_nextsmsg(card); \
> -		} else m = NULL;				\
> +		} else

    Backslash is still missing here.

> +			m = NULL;				\
>   	}
>
>   #define ACTCAPI_CHKSKB if (!skb) {					\
[...]

WBR, Sergei

Re: [PATCH 7/7] drivers: isdn: act2000: capi.c: add macro \ and fix brace

[Sergei Shtylyov]

On 02/07/2015 10:05 PM, Bas Peters wrote:

> This patch adds the \ that was accidentally deleted in patch 2. It also adds a brace after the else statement, which is required due to the fact that the if statement has braces.

    This won't do, fix up the patch #2 please. And please wrap your change log 
at 80 columns or less. And you forgot to sign off anyway. :-)

[...]

WBR, Sergei

Re: [PATCH 7/7] drivers: isdn: act2000: capi.c: add macro \ and fix brace

[Julia Lawall]

On Sat, 7 Feb 2015, Bas Peters wrote:

> This patch adds the \ that was accidentally deleted in patch 2. It also adds a brace after the else statement, which is required due to the fact that the if statement has braces.

You should fix the patch that was incorrect, rather than submitting a 
patch that does something incorrect and then another patch to fix it.  The 
kernel should compile after every patch.

julia

> 
> ---
>  drivers/isdn/act2000/capi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
> index 5d677e6..0043b3c 100644
> --- a/drivers/isdn/act2000/capi.c
> +++ b/drivers/isdn/act2000/capi.c
> @@ -113,8 +113,9 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
>  			m->hdr.cmd.cmd = c;			\
>  			m->hdr.cmd.subcmd = s;			\
>  			m->hdr.msgnum = actcapi_nextsmsg(card); \
> -		} else
> +		} else {					\
>  			m = NULL;				\
> +		}						\
>  	}
>  
>  #define ACTCAPI_CHKSKB if (!skb) {					\
> -- 
> 2.1.0
> 
> 

[PATCH 7/7] drivers: isdn: act2000: capi.c: add macro \ and fix brace

[Bas Peters]

This patch adds the \ that was accidentally deleted in patch 2. It also adds a brace after the else statement, which is required due to the fact that the if statement has braces.

---
 drivers/isdn/act2000/capi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
index 5d677e6..0043b3c 100644
--- a/drivers/isdn/act2000/capi.c
+++ b/drivers/isdn/act2000/capi.c
@@ -113,8 +113,9 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
 			m->hdr.cmd.cmd = c;			\
 			m->hdr.cmd.subcmd = s;			\
 			m->hdr.msgnum = actcapi_nextsmsg(card); \
-		} else
+		} else {					\
 			m = NULL;				\
+		}						\
 	}
 
 #define ACTCAPI_CHKSKB if (!skb) {					\
-- 
2.1.0