[PATCH v2] staging: rtl8712: fix unnecessary elses after return/break in rtl8712_efuse.c

[PATCH v2] staging: rtl8712: fix unnecessary elses after return/break in rtl8712_efuse.c

[Serguey Parkhomovsky]

This patch fixes two unnecessary else conditions that were found by checkpatch.pl.

Signed-off-by: Serguey Parkhomovsky <sergueyparkhomovsky@gmail.com>
---
v2: use negation instead of comparing to false in if statement (suggested by Joe Perches)
 drivers/staging/rtl8712/rtl8712_efuse.c | 40 ++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl8712_efuse.c b/drivers/staging/rtl8712/rtl8712_efuse.c
index c9eeb42..97b7839 100644
--- a/drivers/staging/rtl8712/rtl8712_efuse.c
+++ b/drivers/staging/rtl8712/rtl8712_efuse.c
@@ -414,19 +414,18 @@ u8 r8712_efuse_pg_packet_write(struct _adapter *padapter, const u8 offset,
 					bResult = false;
 			}
 			break;
-		} else { /* write header fail */
-			bResult = false;
-			if (0xFF == efuse_data)
-				return bResult; /* nothing damaged. */
-			/* call rescue procedure */
-			if (fix_header(padapter, efuse_data, efuse_addr) ==
-			    false)
-				return false; /* rescue fail */
-
-			if (++repeat_times > _REPEAT_THRESHOLD_) /* fail */
-				break;
-			/* otherwise, take another risk... */
 		}
+		/* write header fail */
+		bResult = false;
+		if (0xFF == efuse_data)
+			return bResult; /* nothing damaged. */
+		/* call rescue procedure */
+		if (!fix_header(padapter, efuse_data, efuse_addr))
+			return false; /* rescue fail */
+
+		if (++repeat_times > _REPEAT_THRESHOLD_) /* fail */
+			break;
+		/* otherwise, take another risk... */
 	}
 	return bResult;
 }
@@ -541,15 +540,16 @@ u8 r8712_efuse_map_write(struct _adapter *padapter, u16 addr, u16 cnts,
 				}
 				idx++;
 				break;
-			} else {
-				if ((data[idx] != pktdata[i]) || (data[idx+1] !=
-				     pktdata[i+1])) {
-					word_en &= ~BIT(i >> 1);
-					newdata[j++] = data[idx];
-					newdata[j++] = data[idx + 1];
-				}
-				idx += 2;
 			}
+
+			if ((data[idx] != pktdata[i]) || (data[idx+1] !=
+			     pktdata[i+1])) {
+				word_en &= ~BIT(i >> 1);
+				newdata[j++] = data[idx];
+				newdata[j++] = data[idx + 1];
+			}
+			idx += 2;
+
 			if (idx == cnts)
 				break;
 		}
-- 
1.9.3

Re: [PATCH v2] staging: rtl8712: fix unnecessary elses after return/break in rtl8712_efuse.c

[Serguey Parkhomovsky]

> 
> This sort of test is generally better where the
> individual tests are written on separate lines like:
> 
> 			if ((data[idx] != pktdata[i]) ||
> 			    (data[idx+1] != pktdata[i+1])) {
> 

Thanks for all the suggestions, Joe. I'll upload a revised patch soon.

Serguey

Re: [PATCH v2] staging: rtl8712: fix unnecessary elses after return/break in rtl8712_efuse.c

[Joe Perches]

On Tue, 2014-10-07 at 20:42 -0700, Serguey Parkhomovsky wrote:
> This patch fixes two unnecessary else conditions that were found by checkpatch.pl.
[]
> diff --git a/drivers/staging/rtl8712/rtl8712_efuse.c b/drivers/staging/rtl8712/rtl8712_efuse.c
[]
> @@ -541,15 +540,16 @@ u8 r8712_efuse_map_write(struct _adapter *padapter, u16 addr, u16 cnts,
[]
> +
> +			if ((data[idx] != pktdata[i]) || (data[idx+1] !=
> +			     pktdata[i+1])) {

Just an fyi: (and I know you didn't introduce this)

Maximal fill to 80 columns is not always the most
readable for humans.

This sort of test is generally better where the
individual tests are written on separate lines like:

			if ((data[idx] != pktdata[i]) ||
			    (data[idx+1] != pktdata[i+1])) {