[PATCH 0/4] Fix coding style of if statement

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/4] Fix coding style of if statement
@ 2014-05-17 16:30 Masaru Nomura
  2014-05-17 16:30 ` [PATCH 1/4] staging: dgnc: Fix indenting of if-else statement Masaru Nomura
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Masaru Nomura @ 2014-05-17 16:30 UTC (permalink / raw)
  To: lidza.louina, markh
  Cc: gregkh, driverdev-devel, devel, linux-kernel, Masaru Nomura

The following pateches fix the errors and warnings in 
dgnc_neo.c and dgnc_tty.c to meet kernel coding style.

ERROR: else should follow close brace '}'
ERROR: that open brace { should be on the previous line
WARNING: line over 80 characters
WARNING: braces {} are not necessary for single statement blocks

Masaru Nomura (4):
  staging: dgnc: Fix indenting of if-else statement
  staging: dgnc: dgnc_neo: Fix indenting of if statement
  staging: dgnc: dgnc_neo: Fix conditional part of if statement
  staging: dgnc: Remove unnecessary braces

 drivers/staging/dgnc/dgnc_neo.c |   67 ++++++++++++++-------------------------
 drivers/staging/dgnc/dgnc_tty.c |   42 ++++++++----------------
 2 files changed, 38 insertions(+), 71 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/4] staging: dgnc: Fix indenting of if-else statement
  2014-05-17 16:30 [PATCH 0/4] Fix coding style of if statement Masaru Nomura
@ 2014-05-17 16:30 ` Masaru Nomura
  2014-05-17 16:30 ` [PATCH 2/4] staging: dgnc: dgnc_neo: Fix indenting of if statement Masaru Nomura
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Masaru Nomura @ 2014-05-17 16:30 UTC (permalink / raw)
  To: lidza.louina, markh
  Cc: gregkh, driverdev-devel, devel, linux-kernel, Masaru Nomura

Fix indenting of if-else statement in dgnc_neo.c and dgnc_tty.c
so that following else-if or else statement meets coding style.

Signed-off-by: Masaru Nomura <massa.nomura@gmail.com>
---
 drivers/staging/dgnc/dgnc_neo.c |   48 +++++++++++++--------------------------
 drivers/staging/dgnc/dgnc_tty.c |   36 ++++++++++-------------------
 2 files changed, 28 insertions(+), 56 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
index cf22c7b..e87cf49 100644
--- a/drivers/staging/dgnc/dgnc_neo.c
+++ b/drivers/staging/dgnc/dgnc_neo.c
@@ -485,8 +485,7 @@ static inline void neo_parse_isr(struct dgnc_board *brd, uint port)
 					DGNC_UNLOCK(ch->ch_lock, lock_flags);
 				}
 				DPR_INTR(("Port %d. XON detected in incoming data\n", port));
-			}
-			else if (cause == UART_17158_XOFF_DETECT) {
+			} else if (cause == UART_17158_XOFF_DETECT) {
 				if (!(brd->channels[port]->ch_flags & CH_STOP)) {
 					DGNC_LOCK(ch->ch_lock, lock_flags);
 					ch->ch_flags |= CH_STOP;
@@ -511,8 +510,7 @@ static inline void neo_parse_isr(struct dgnc_board *brd, uint port)
 					DGNC_LOCK(ch->ch_lock, lock_flags);
 					ch->ch_mostat |= UART_MCR_RTS;
 					DGNC_UNLOCK(ch->ch_lock, lock_flags);
-				}
-				else {
+				} else {
 					DGNC_LOCK(ch->ch_lock, lock_flags);
 					ch->ch_mostat &= ~(UART_MCR_RTS);
 					DGNC_UNLOCK(ch->ch_lock, lock_flags);
@@ -522,8 +520,7 @@ static inline void neo_parse_isr(struct dgnc_board *brd, uint port)
 					DGNC_LOCK(ch->ch_lock, lock_flags);
 					ch->ch_mostat |= UART_MCR_DTR;
 					DGNC_UNLOCK(ch->ch_lock, lock_flags);
-				}
-				else {
+				} else {
 					DGNC_LOCK(ch->ch_lock, lock_flags);
 					ch->ch_mostat &= ~(UART_MCR_DTR);
 					DGNC_UNLOCK(ch->ch_lock, lock_flags);
@@ -624,8 +621,7 @@ static inline void neo_parse_lsr(struct dgnc_board *brd, uint port)
 
 		/* Transfer data (if any) from Write Queue -> UART. */
 		neo_copy_data_from_queue_to_uart(ch);
-	}
-	else if (linestatus & UART_17158_TX_AND_FIFO_CLR) {
+	} else if (linestatus & UART_17158_TX_AND_FIFO_CLR) {
 		brd->intr_tx++;
 		ch->ch_intr_tx++;
 		DGNC_LOCK(ch->ch_lock, lock_flags);
@@ -834,8 +830,7 @@ static void neo_param(struct tty_struct *tty)
 
 	if (ch->ch_c_cflag & CREAD) {
 		ier |= (UART_IER_RDI | UART_IER_RLSI);
-	}
-	else {
+	} else {
 		ier &= ~(UART_IER_RDI | UART_IER_RLSI);
 	}
 
@@ -848,8 +843,7 @@ static void neo_param(struct tty_struct *tty)
 		!(ch->ch_c_cflag & CLOCAL))
 	{
 		ier |= UART_IER_MSI;
-	}
-	else {
+	} else {
 		ier &= ~UART_IER_MSI;
 	}
 
@@ -863,29 +857,25 @@ static void neo_param(struct tty_struct *tty)
 
 	if (ch->ch_digi.digi_flags & CTSPACE || ch->ch_c_cflag & CRTSCTS) {
 		neo_set_cts_flow_control(ch);
-	}
-	else if (ch->ch_c_iflag & IXON) {
+	} else if (ch->ch_c_iflag & IXON) {
 		/* If start/stop is set to disable, then we should disable flow control */
 		if ((ch->ch_startc == _POSIX_VDISABLE) || (ch->ch_stopc == _POSIX_VDISABLE))
 			neo_set_no_output_flow_control(ch);
 		else
 			neo_set_ixon_flow_control(ch);
-	}
-	else {
+	} else {
 		neo_set_no_output_flow_control(ch);
 	}
 
 	if (ch->ch_digi.digi_flags & RTSPACE || ch->ch_c_cflag & CRTSCTS) {
 		neo_set_rts_flow_control(ch);
-	}
-	else if (ch->ch_c_iflag & IXOFF) {
+	} else if (ch->ch_c_iflag & IXOFF) {
 		/* If start/stop is set to disable, then we should disable flow control */
 		if ((ch->ch_startc == _POSIX_VDISABLE) || (ch->ch_stopc == _POSIX_VDISABLE))
 			neo_set_no_input_flow_control(ch);
 		else
 			neo_set_ixoff_flow_control(ch);
-	}
-	else {
+	} else {
 		neo_set_no_input_flow_control(ch);
 	}
 
@@ -1227,8 +1217,7 @@ static void neo_copy_data_from_uart_to_queue(struct channel_t *ch)
 		 */
 		if ((ch->ch_bd->dvid & 0xf0) >= UART_XR17E158_DVID) {
 			total -= 1;
-		}
-		else {
+		} else {
 			total -= 3;
 		}
 	}
@@ -1435,8 +1424,7 @@ static int neo_drain(struct tty_struct *tty, uint seconds)
 	/* If ret is non-zero, user ctrl-c'ed us */
 	if (rc) {
 		DPR_IOCTL(("%d Drain - User ctrl c'ed\n", __LINE__));
-	}
-	else {
+	} else {
 		DPR_IOCTL(("%d Drain wait finished.\n", __LINE__));
 	}
 
@@ -1468,8 +1456,7 @@ static void neo_flush_uart_write(struct channel_t *ch)
 		if (tmp & 4) {
 			DPR_IOCTL(("Still flushing TX UART... i: %d\n", i));
 			udelay(10);
-		}
-		else
+		} else
 			break;
 	}
 
@@ -1501,8 +1488,7 @@ static void neo_flush_uart_read(struct channel_t *ch)
 		if (tmp & 2) {
 			DPR_IOCTL(("Still flushing RX UART... i: %d\n", i));
 			udelay(10);
-		}
-		else
+		} else
 			break;
 	}
 }
@@ -1598,8 +1584,7 @@ static void neo_copy_data_from_queue_to_uart(struct channel_t *ch)
 		}
 
 		n = UART_17158_TX_FIFOSIZE - ch->ch_t_tlevel;
-	}
-	else {
+	} else {
 		n = UART_17158_TX_FIFOSIZE - readb(&ch->ch_neo_uart->tfifo);
 	}
 
@@ -1963,8 +1948,7 @@ static void neo_vpd(struct dgnc_board *brd)
 		||  (brd->vpd[0x7F] != 0x78))   /* small resource end tag */
 	{
 		memset(brd->vpd, '\0', NEO_VPD_IMAGESIZE);
-	}
-	else {
+	} else {
 		/* Search for the serial number */
 		for (i = 0; i < NEO_VPD_IMAGEBYTES - 3; i++) {
 			if (brd->vpd[i] == 'S' && brd->vpd[i + 1] == 'N') {
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index f0b17c3..e432c04 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -803,8 +803,7 @@ void dgnc_input(struct channel_t *ch)
 				else
 					tty_insert_flip_char(tp->port, *(ch->ch_rqueue + tail + i), TTY_NORMAL);
 			}
-		}
-		else {
+		} else {
 			tty_insert_flip_string(tp->port, ch->ch_rqueue + tail, s);
 		}
 
@@ -1267,12 +1266,10 @@ static int dgnc_tty_open(struct tty_struct *tty, struct file *file)
 	if (!IS_PRINT(minor)) {
 		un = &brd->channels[PORT_NUM(minor)]->ch_tun;
 		un->un_type = DGNC_SERIAL;
-	}
-	else if (IS_PRINT(minor)) {
+	} else if (IS_PRINT(minor)) {
 		un = &brd->channels[PORT_NUM(minor)]->ch_pun;
 		un->un_type = DGNC_PRINT;
-	}
-	else {
+	} else {
 		DGNC_UNLOCK(ch->ch_lock, lock_flags);
 		DPR_OPEN(("%d Unknown TYPE!\n", __LINE__));
 		return -ENXIO;
@@ -1507,8 +1504,7 @@ static int dgnc_block_til_ready(struct tty_struct *tty, struct file *file, struc
 				DPR_OPEN(("%d: ch_flags: %x\n", __LINE__, ch->ch_flags));
 				break;
 			}
-		}
-		else {
+		} else {
 			sleep_on_un_flags = 1;
 		}
 
@@ -1550,8 +1546,7 @@ static int dgnc_block_til_ready(struct tty_struct *tty, struct file *file, struc
 		if (sleep_on_un_flags) {
 			retval = wait_event_interruptible(un->un_flags_wait,
 				(old_flags != (ch->ch_tun.un_flags | ch->ch_pun.un_flags)));
-		}
-		else {
+		} else {
 			retval = wait_event_interruptible(ch->ch_flags_wait,
 				(old_flags != ch->ch_flags));
 		}
@@ -1748,8 +1743,7 @@ static void dgnc_tty_close(struct tty_struct *tty, struct file *file)
 
 		/* Turn off UART interrupts for this port */
 		ch->ch_bd->bd_ops->uart_off(ch);
-	}
-	else {
+	} else {
 		/*
 		 * turn off print device when closing print device.
 		 */
@@ -1867,12 +1861,10 @@ static int dgnc_maxcps_room(struct tty_struct *tty, int bytes_available)
 			/* buffer is empty */
 			ch->ch_cpstime = current_time;	    /* reset ch_cpstime */
 			cps_limit = ch->ch_digi.digi_bufsize;
-		}
-		else if (ch->ch_cpstime < buffer_time) {
+		} else if (ch->ch_cpstime < buffer_time) {
 			/* still room in the buffer */
 			cps_limit = ((buffer_time - ch->ch_cpstime) * ch->ch_digi.digi_maxcps) / HZ;
-		}
-		else {
+		} else {
 			/* no room in the buffer */
 			cps_limit = 0;
 		}
@@ -1931,8 +1923,7 @@ static int dgnc_tty_write_room(struct tty_struct *tty)
 		if (!(ch->ch_flags & CH_PRON))
 			ret -= ch->ch_digi.digi_onlen;
 		ret -= ch->ch_digi.digi_offlen;
-	}
-	else {
+	} else {
 		if (ch->ch_flags & CH_PRON)
 			ret -= ch->ch_digi.digi_offlen;
 	}
@@ -2560,15 +2551,13 @@ static int dgnc_set_modem_info(struct tty_struct *tty, unsigned int command, uns
 
 		if (arg & TIOCM_RTS) {
 			ch->ch_mostat |= UART_MCR_RTS;
-		}
-		else {
+		} else {
 			ch->ch_mostat &= ~(UART_MCR_RTS);
 		}
 
 		if (arg & TIOCM_DTR) {
 			ch->ch_mostat |= UART_MCR_DTR;
-		}
-		else {
+		} else {
 			ch->ch_mostat &= ~(UART_MCR_DTR);
 		}
 
@@ -3279,8 +3268,7 @@ static int dgnc_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
 				return -EINTR;
 			}
 			DGNC_LOCK(ch->ch_lock, lock_flags);
-		}
-		else {
+		} else {
 			tty_ldisc_flush(tty);
 		}
 		/* fall thru */
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/4] staging: dgnc: dgnc_neo: Fix indenting of if statement
  2014-05-17 16:30 [PATCH 0/4] Fix coding style of if statement Masaru Nomura
  2014-05-17 16:30 ` [PATCH 1/4] staging: dgnc: Fix indenting of if-else statement Masaru Nomura
@ 2014-05-17 16:30 ` Masaru Nomura
  2014-05-17 16:30 ` [PATCH 3/4] staging: dgnc: dgnc_neo: Fix conditional part " Masaru Nomura
  2014-05-17 16:30 ` [PATCH 4/4] staging: dgnc: Remove unnecessary braces Masaru Nomura
  3 siblings, 0 replies; 8+ messages in thread
From: Masaru Nomura @ 2014-05-17 16:30 UTC (permalink / raw)
  To: lidza.louina, markh
  Cc: gregkh, driverdev-devel, devel, linux-kernel, Masaru Nomura

Fix indenting of if statement in dgnc_neo.c so that an open brace
follows if-condition part to meet kernel coding style.

Signed-off-by: Masaru Nomura <massa.nomura@gmail.com>
---
 drivers/staging/dgnc/dgnc_neo.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
index e87cf49..8988346 100644
--- a/drivers/staging/dgnc/dgnc_neo.c
+++ b/drivers/staging/dgnc/dgnc_neo.c
@@ -840,8 +840,7 @@ static void neo_param(struct tty_struct *tty)
 	 */
 	if ((ch->ch_digi.digi_flags & CTSPACE) || (ch->ch_digi.digi_flags & RTSPACE) ||
 		(ch->ch_c_cflag & CRTSCTS) || !(ch->ch_digi.digi_flags & DIGI_FORCEDCD) ||
-		!(ch->ch_c_cflag & CLOCAL))
-	{
+		!(ch->ch_c_cflag & CLOCAL)) {
 		ier |= UART_IER_MSI;
 	} else {
 		ier &= ~UART_IER_MSI;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/4] staging: dgnc: dgnc_neo: Fix conditional part of if statement
  2014-05-17 16:30 [PATCH 0/4] Fix coding style of if statement Masaru Nomura
  2014-05-17 16:30 ` [PATCH 1/4] staging: dgnc: Fix indenting of if-else statement Masaru Nomura
  2014-05-17 16:30 ` [PATCH 2/4] staging: dgnc: dgnc_neo: Fix indenting of if statement Masaru Nomura
@ 2014-05-17 16:30 ` Masaru Nomura
  2014-05-17 19:39   ` Dan Carpenter
  2014-05-17 16:30 ` [PATCH 4/4] staging: dgnc: Remove unnecessary braces Masaru Nomura
  3 siblings, 1 reply; 8+ messages in thread
From: Masaru Nomura @ 2014-05-17 16:30 UTC (permalink / raw)
  To: lidza.louina, markh
  Cc: gregkh, driverdev-devel, devel, linux-kernel, Masaru Nomura

Fix line over 80 characters of if-condition part and also
indent the lines to tell the difference between the condition and
body of the if statement.

Then I think we can keep the readability and meet coding style
with this change.

Signed-off-by: Masaru Nomura <massa.nomura@gmail.com>
---
 drivers/staging/dgnc/dgnc_neo.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
index 8988346..c211f9f 100644
--- a/drivers/staging/dgnc/dgnc_neo.c
+++ b/drivers/staging/dgnc/dgnc_neo.c
@@ -838,9 +838,11 @@ static void neo_param(struct tty_struct *tty)
 	 * Have the UART interrupt on modem signal changes ONLY when
 	 * we are in hardware flow control mode, or CLOCAL/FORCEDCD is not set.
 	 */
-	if ((ch->ch_digi.digi_flags & CTSPACE) || (ch->ch_digi.digi_flags & RTSPACE) ||
-		(ch->ch_c_cflag & CRTSCTS) || !(ch->ch_digi.digi_flags & DIGI_FORCEDCD) ||
-		!(ch->ch_c_cflag & CLOCAL)) {
+	if ((ch->ch_digi.digi_flags & CTSPACE)
+			|| (ch->ch_digi.digi_flags & RTSPACE)
+			|| (ch->ch_c_cflag & CRTSCTS)
+			|| !(ch->ch_digi.digi_flags & DIGI_FORCEDCD)
+			|| !(ch->ch_c_cflag & CLOCAL)) {
 		ier |= UART_IER_MSI;
 	} else {
 		ier &= ~UART_IER_MSI;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/4] staging: dgnc: dgnc_neo: Fix conditional part of if statement
  2014-05-17 16:30 ` [PATCH 3/4] staging: dgnc: dgnc_neo: Fix conditional part " Masaru Nomura
@ 2014-05-17 19:39   ` Dan Carpenter
  2014-05-17 21:14     ` Masaru Nomura
  0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2014-05-17 19:39 UTC (permalink / raw)
  To: Masaru Nomura
  Cc: lidza.louina, markh, devel, gregkh, driverdev-devel, linux-kernel

On Sat, May 17, 2014 at 05:30:53PM +0100, Masaru Nomura wrote:
> diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
> index 8988346..c211f9f 100644
> --- a/drivers/staging/dgnc/dgnc_neo.c
> +++ b/drivers/staging/dgnc/dgnc_neo.c
> @@ -838,9 +838,11 @@ static void neo_param(struct tty_struct *tty)
>  	 * Have the UART interrupt on modem signal changes ONLY when
>  	 * we are in hardware flow control mode, or CLOCAL/FORCEDCD is not set.
>  	 */
> -	if ((ch->ch_digi.digi_flags & CTSPACE) || (ch->ch_digi.digi_flags & RTSPACE) ||
> -		(ch->ch_c_cflag & CRTSCTS) || !(ch->ch_digi.digi_flags & DIGI_FORCEDCD) ||
> -		!(ch->ch_c_cflag & CLOCAL)) {
> +	if ((ch->ch_digi.digi_flags & CTSPACE)
> +			|| (ch->ch_digi.digi_flags & RTSPACE)
> +			|| (ch->ch_c_cflag & CRTSCTS)
> +			|| !(ch->ch_digi.digi_flags & DIGI_FORCEDCD)
> +			|| !(ch->ch_c_cflag & CLOCAL)) {

This isn't the right way.  Write it like this:

	if ((ch->ch_digi.digi_flags & CTSPACE) ||
	    (ch->ch_digi.digi_flags & RTSPACE) ||
	    (ch->ch_c_cflag & CRTSCTS) ||
	    !(ch->ch_digi.digi_flags & DIGI_FORCEDCD) ||
	    !(ch->ch_c_cflag & CLOCAL)) {
		ier |= UART_IER_MSI;
	else
		ier &= ~UART_IER_MSI;

1) The "||" operation goes at the end of the line.
2) The conditions are all in a line.  The indenting is

[tab][space][space][space][space](ch->ch_digi.digi_f...

Also just fold this patch and [patch 2/4] together into one patch.  We
don't need two patches to fix one if statement.

The one thing per patch rule is a bit tricky.  It means that you have to
say which one thing you are fixing.  Don't say "I am fixing three
things."  Say "I am fixing one if statement".

regards,
dan carpenter



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/4] staging: dgnc: dgnc_neo: Fix conditional part of if statement
  2014-05-17 19:39   ` Dan Carpenter
@ 2014-05-17 21:14     ` Masaru Nomura
  2014-05-17 21:23       ` Dan Carpenter
  0 siblings, 1 reply; 8+ messages in thread
From: Masaru Nomura @ 2014-05-17 21:14 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Lidza Louina, markh, devel, gregkh, driverdev-devel, linux-kernel

Hi Dan,

Thank you for your detailed explanation!

> This isn't the right way.  Write it like this:
>
>         if ((ch->ch_digi.digi_flags & CTSPACE) ||
>             (ch->ch_digi.digi_flags & RTSPACE) ||
>             (ch->ch_c_cflag & CRTSCTS) ||
>             !(ch->ch_digi.digi_flags & DIGI_FORCEDCD) ||
>             !(ch->ch_c_cflag & CLOCAL)) {
>                 ier |= UART_IER_MSI;
>         else
>                 ier &= ~UART_IER_MSI;
>
> 1) The "||" operation goes at the end of the line.
> 2) The conditions are all in a line.  The indenting is
>
> [tab][space][space][space][space](ch->ch_digi.digi_f...

Sure, I'll modify this.

> Also just fold this patch and [patch 2/4] together into one patch.  We
> don't need two patches to fix one if statement.
>
> The one thing per patch rule is a bit tricky.  It means that you have to
> say which one thing you are fixing.  Don't say "I am fixing three
> things."  Say "I am fixing one if statement".

So in this case, do you think I could fold all four patches into one?
As I just worked on the 'same' if statements and divided them into
four patches, this could be the case. (but I'm not sure yet...)

Also, should I put v[number] for the modified patches?
Say [PATCH v2] staging: ...

Thank you,
Masaru

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/4] staging: dgnc: dgnc_neo: Fix conditional part of if statement
  2014-05-17 21:14     ` Masaru Nomura
@ 2014-05-17 21:23       ` Dan Carpenter
  0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2014-05-17 21:23 UTC (permalink / raw)
  To: Masaru Nomura; +Cc: devel, Lidza Louina, driverdev-devel, linux-kernel, gregkh

On Sat, May 17, 2014 at 10:14:52PM +0100, Masaru Nomura wrote:
> > Also just fold this patch and [patch 2/4] together into one patch.  We
> > don't need two patches to fix one if statement.
> >
> > The one thing per patch rule is a bit tricky.  It means that you have to
> > say which one thing you are fixing.  Don't say "I am fixing three
> > things."  Say "I am fixing one if statement".
> 
> So in this case, do you think I could fold all four patches into one?

No.  That would be more than one thing per patch.  It looks like this:

[patch 1/3] staging: dgnc: dgnc_neo: put else statements on the right line
[patch 2/3] staging: dgnc: dgnc_neo: clean up ugly one very ugly condition
[patch 3/3] staging: dgnc: dgnc_neo: remove extra curly braces

Each patch does one thing.  Cleaning up the whole file is too big so it
doesn't count as doing one thing only.

> As I just worked on the 'same' if statements and divided them into
> four patches, this could be the case. (but I'm not sure yet...)
> 
> Also, should I put v[number] for the modified patches?
> Say [PATCH v2] staging: ...

Yes.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 4/4] staging: dgnc: Remove unnecessary braces
  2014-05-17 16:30 [PATCH 0/4] Fix coding style of if statement Masaru Nomura
                   ` (2 preceding siblings ...)
  2014-05-17 16:30 ` [PATCH 3/4] staging: dgnc: dgnc_neo: Fix conditional part " Masaru Nomura
@ 2014-05-17 16:30 ` Masaru Nomura
  3 siblings, 0 replies; 8+ messages in thread
From: Masaru Nomura @ 2014-05-17 16:30 UTC (permalink / raw)
  To: lidza.louina, markh
  Cc: gregkh, driverdev-devel, devel, linux-kernel, Masaru Nomura

Remove unnecessary braces of if-else statements in dgnc_neo.c and
dgnc_tty.c to meet kernel coding style.

Signed-off-by: Masaru Nomura <massa.nomura@gmail.com>
---
 drivers/staging/dgnc/dgnc_neo.c |   20 ++++++++------------
 drivers/staging/dgnc/dgnc_tty.c |   10 ++++------
 2 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
index c211f9f..c4ab17b 100644
--- a/drivers/staging/dgnc/dgnc_neo.c
+++ b/drivers/staging/dgnc/dgnc_neo.c
@@ -828,11 +828,10 @@ static void neo_param(struct tty_struct *tty)
 	if (uart_lcr != lcr)
 		writeb(lcr, &ch->ch_neo_uart->lcr);
 
-	if (ch->ch_c_cflag & CREAD) {
+	if (ch->ch_c_cflag & CREAD)
 		ier |= (UART_IER_RDI | UART_IER_RLSI);
-	} else {
+	else
 		ier &= ~(UART_IER_RDI | UART_IER_RLSI);
-	}
 
 	/*
 	 * Have the UART interrupt on modem signal changes ONLY when
@@ -842,11 +841,10 @@ static void neo_param(struct tty_struct *tty)
 			|| (ch->ch_digi.digi_flags & RTSPACE)
 			|| (ch->ch_c_cflag & CRTSCTS)
 			|| !(ch->ch_digi.digi_flags & DIGI_FORCEDCD)
-			|| !(ch->ch_c_cflag & CLOCAL)) {
+			|| !(ch->ch_c_cflag & CLOCAL))
 		ier |= UART_IER_MSI;
-	} else {
+	else
 		ier &= ~UART_IER_MSI;
-	}
 
 	ier |= UART_IER_THRI;
 
@@ -1216,11 +1214,10 @@ static void neo_copy_data_from_uart_to_queue(struct channel_t *ch)
 		 * The count can be any where from 0-3 bytes "off".
 		 * Bizarre, but true.
 		 */
-		if ((ch->ch_bd->dvid & 0xf0) >= UART_XR17E158_DVID) {
+		if ((ch->ch_bd->dvid & 0xf0) >= UART_XR17E158_DVID)
 			total -= 1;
-		} else {
+		else
 			total -= 3;
-		}
 	}
 
 
@@ -1423,11 +1420,10 @@ static int neo_drain(struct tty_struct *tty, uint seconds)
 	rc = wait_event_interruptible(un->un_flags_wait, ((un->un_flags & UN_EMPTY) == 0));
 
 	/* If ret is non-zero, user ctrl-c'ed us */
-	if (rc) {
+	if (rc)
 		DPR_IOCTL(("%d Drain - User ctrl c'ed\n", __LINE__));
-	} else {
+	else
 		DPR_IOCTL(("%d Drain wait finished.\n", __LINE__));
-	}
 
 	return rc;
 }
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index e432c04..cda2254 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -2549,17 +2549,15 @@ static int dgnc_set_modem_info(struct tty_struct *tty, unsigned int command, uns
 
 	case TIOCMSET:
 
-		if (arg & TIOCM_RTS) {
+		if (arg & TIOCM_RTS)
 			ch->ch_mostat |= UART_MCR_RTS;
-		} else {
+		else
 			ch->ch_mostat &= ~(UART_MCR_RTS);
-		}
 
-		if (arg & TIOCM_DTR) {
+		if (arg & TIOCM_DTR)
 			ch->ch_mostat |= UART_MCR_DTR;
-		} else {
+		else
 			ch->ch_mostat &= ~(UART_MCR_DTR);
-		}
 
 		break;
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-05-17 21:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-17 16:30 [PATCH 0/4] Fix coding style of if statement Masaru Nomura
2014-05-17 16:30 ` [PATCH 1/4] staging: dgnc: Fix indenting of if-else statement Masaru Nomura
2014-05-17 16:30 ` [PATCH 2/4] staging: dgnc: dgnc_neo: Fix indenting of if statement Masaru Nomura
2014-05-17 16:30 ` [PATCH 3/4] staging: dgnc: dgnc_neo: Fix conditional part " Masaru Nomura
2014-05-17 19:39   ` Dan Carpenter
2014-05-17 21:14     ` Masaru Nomura
2014-05-17 21:23       ` Dan Carpenter
2014-05-17 16:30 ` [PATCH 4/4] staging: dgnc: Remove unnecessary braces Masaru Nomura

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox