* [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
* [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
* 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
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