diff mbox series

[1/4] staging: wilc1000: remove use of infinite loop conditions

Message ID 20200117160157.8706-2-ajay.kathat@microchip.com
State New
Headers show
Series staging: wilc1000: handle few full driver review comments | expand

Commit Message

Ajay Singh Jan. 17, 2020, 10:31 a.m. UTC 
From: Ajay Singh <ajay.kathat@microchip.com>

Avoid the use of 'while (1)' infinite loop conditions. It's not
recommended to have infinite loop in kernel code because a small bug can
cause never ending loops so use terminator condition as suggested in
full driver review [1].

[1]. https://lore.kernel.org/linux-wireless/20191023100313.52B3F606CF@smtp.codeaurora.org/

Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 drivers/staging/wilc1000/netdev.c   |   7 +-
 drivers/staging/wilc1000/wlan.c     |  79 ++++++---------
 drivers/staging/wilc1000/wlan_cfg.c | 144 ++++++++++------------------
 3 files changed, 84 insertions(+), 146 deletions(-)
diff mbox series

Patch

diff --git a/drivers/staging/wilc1000/netdev.c b/drivers/staging/wilc1000/netdev.c
index 3fd8e008f733..960dbc8d5173 100644
--- a/drivers/staging/wilc1000/netdev.c
+++ b/drivers/staging/wilc1000/netdev.c
@@ -837,7 +837,7 @@  static const struct net_device_ops wilc_netdev_ops = {
 void wilc_netdev_cleanup(struct wilc *wilc)
 {
 	struct wilc_vif *vif;
-	int srcu_idx;
+	int srcu_idx, ifc_cnt = 0;
 
 	if (!wilc)
 		return;
@@ -858,7 +858,7 @@  void wilc_netdev_cleanup(struct wilc *wilc)
 	flush_workqueue(wilc->hif_workqueue);
 	destroy_workqueue(wilc->hif_workqueue);
 
-	do {
+	while (ifc_cnt < WILC_NUM_CONCURRENT_IFC) {
 		mutex_lock(&wilc->vif_mutex);
 		if (wilc->vif_num <= 0) {
 			mutex_unlock(&wilc->vif_mutex);
@@ -871,7 +871,8 @@  void wilc_netdev_cleanup(struct wilc *wilc)
 		wilc->vif_num--;
 		mutex_unlock(&wilc->vif_mutex);
 		synchronize_srcu(&wilc->srcu);
-	} while (1);
+		ifc_cnt++;
+	}
 
 	wilc_wlan_cfg_deinit(wilc);
 	wlan_deinit_locks(wilc);
diff --git a/drivers/staging/wilc1000/wlan.c b/drivers/staging/wilc1000/wlan.c
index ba5446724c93..c32af7076012 100644
--- a/drivers/staging/wilc1000/wlan.c
+++ b/drivers/staging/wilc1000/wlan.c
@@ -499,37 +499,31 @@  int wilc_wlan_handle_txq(struct wilc *wilc, u32 *txq_count)
 	wilc_wlan_txq_filter_dup_tcp_ack(dev);
 	i = 0;
 	sum = 0;
-	do {
-		if (tqe && (i < (WILC_VMM_TBL_SIZE - 1))) {
-			if (tqe->type == WILC_CFG_PKT)
-				vmm_sz = ETH_CONFIG_PKT_HDR_OFFSET;
-
-			else if (tqe->type == WILC_NET_PKT)
-				vmm_sz = ETH_ETHERNET_HDR_OFFSET;
-
-			else
-				vmm_sz = HOST_HDR_OFFSET;
+	while (tqe && (i < (WILC_VMM_TBL_SIZE - 1))) {
+		if (tqe->type == WILC_CFG_PKT)
+			vmm_sz = ETH_CONFIG_PKT_HDR_OFFSET;
+		else if (tqe->type == WILC_NET_PKT)
+			vmm_sz = ETH_ETHERNET_HDR_OFFSET;
+		else
+			vmm_sz = HOST_HDR_OFFSET;
 
-			vmm_sz += tqe->buffer_size;
+		vmm_sz += tqe->buffer_size;
 
-			if (vmm_sz & 0x3)
-				vmm_sz = (vmm_sz + 4) & ~0x3;
+		if (vmm_sz & 0x3)
+			vmm_sz = (vmm_sz + 4) & ~0x3;
 
-			if ((sum + vmm_sz) > WILC_TX_BUFF_SIZE)
-				break;
+		if ((sum + vmm_sz) > WILC_TX_BUFF_SIZE)
+			break;
 
-			vmm_table[i] = vmm_sz / 4;
-			if (tqe->type == WILC_CFG_PKT)
-				vmm_table[i] |= BIT(10);
-			cpu_to_le32s(&vmm_table[i]);
+		vmm_table[i] = vmm_sz / 4;
+		if (tqe->type == WILC_CFG_PKT)
+			vmm_table[i] |= BIT(10);
+		cpu_to_le32s(&vmm_table[i]);
 
-			i++;
-			sum += vmm_sz;
-			tqe = wilc_wlan_txq_get_next(wilc, tqe);
-		} else {
-			break;
-		}
-	} while (1);
+		i++;
+		sum += vmm_sz;
+		tqe = wilc_wlan_txq_get_next(wilc, tqe);
+	}
 
 	if (i == 0)
 		goto out;
@@ -594,12 +588,8 @@  int wilc_wlan_handle_txq(struct wilc *wilc, u32 *txq_count)
 				break;
 			reg &= ~BIT(0);
 			ret = func->hif_write_reg(wilc, WILC_HOST_TX_CTRL, reg);
-			if (!ret)
-				break;
-			break;
 		}
-		break;
-	} while (1);
+	} while (0);
 
 	if (!ret)
 		goto out_release_bus;
@@ -725,9 +715,7 @@  static void wilc_wlan_handle_rx_buff(struct wilc *wilc, u8 *buffer, int size)
 			}
 		}
 		offset += tp_len;
-		if (offset >= size)
-			break;
-	} while (1);
+	} while (offset < size);
 }
 
 static void wilc_wlan_handle_rxq(struct wilc *wilc)
@@ -736,11 +724,7 @@  static void wilc_wlan_handle_rxq(struct wilc *wilc)
 	u8 *buffer;
 	struct rxq_entry_t *rqe;
 
-	do {
-		if (wilc->quit) {
-			complete(&wilc->cfg_event);
-			break;
-		}
+	while (!wilc->quit) {
 		rqe = wilc_wlan_rxq_remove(wilc);
 		if (!rqe)
 			break;
@@ -750,7 +734,9 @@  static void wilc_wlan_handle_rxq(struct wilc *wilc)
 		wilc_wlan_handle_rx_buff(wilc, buffer, size);
 
 		kfree(rqe);
-	} while (1);
+	}
+	if (wilc->quit)
+		complete(&wilc->cfg_event);
 }
 
 static void wilc_unknown_isr_ext(struct wilc *wilc)
@@ -969,21 +955,14 @@  void wilc_wlan_cleanup(struct net_device *dev)
 	struct wilc *wilc = vif->wilc;
 
 	wilc->quit = 1;
-	do {
-		tqe = wilc_wlan_txq_remove_from_head(dev);
-		if (!tqe)
-			break;
+	while ((tqe = wilc_wlan_txq_remove_from_head(dev))) {
 		if (tqe->tx_complete_func)
 			tqe->tx_complete_func(tqe->priv, 0);
 		kfree(tqe);
-	} while (1);
+	}
 
-	do {
-		rqe = wilc_wlan_rxq_remove(wilc);
-		if (!rqe)
-			break;
+	while ((rqe = wilc_wlan_rxq_remove(wilc)))
 		kfree(rqe);
-	} while (1);
 
 	kfree(wilc->rx_buffer);
 	wilc->rx_buffer = NULL;
diff --git a/drivers/staging/wilc1000/wlan_cfg.c b/drivers/staging/wilc1000/wlan_cfg.c
index 2538435b82fd..fe2a7ed8e5cd 100644
--- a/drivers/staging/wilc1000/wlan_cfg.c
+++ b/drivers/staging/wilc1000/wlan_cfg.c
@@ -137,6 +137,7 @@  static void wilc_wlan_parse_response_frame(struct wilc *wl, u8 *info, int size)
 {
 	u16 wid;
 	u32 len = 0, i = 0;
+	struct wilc_cfg *cfg = &wl->cfg;
 
 	while (size > 0) {
 		i = 0;
@@ -144,63 +145,42 @@  static void wilc_wlan_parse_response_frame(struct wilc *wl, u8 *info, int size)
 
 		switch (FIELD_GET(WILC_WID_TYPE, wid)) {
 		case WID_CHAR:
-			do {
-				if (wl->cfg.b[i].id == WID_NIL)
-					break;
-
-				if (wl->cfg.b[i].id == wid) {
-					wl->cfg.b[i].val = info[4];
-					break;
-				}
+			while (cfg->b[i].id != WID_NIL && cfg->b[i].id != wid)
 				i++;
-			} while (1);
+
+			if (cfg->b[i].id == wid)
+				cfg->b[i].val = info[4];
+
 			len = 3;
 			break;
 
 		case WID_SHORT:
-			do {
-				struct wilc_cfg_hword *hw = &wl->cfg.hw[i];
+			while (cfg->hw[i].id != WID_NIL && cfg->hw[i].id != wid)
+				i++;
 
-				if (hw->id == WID_NIL)
-					break;
+			if (cfg->hw[i].id == wid)
+				cfg->hw[i].val = get_unaligned_le16(&info[4]);
 
-				if (hw->id == wid) {
-					hw->val = get_unaligned_le16(&info[4]);
-					break;
-				}
-				i++;
-			} while (1);
 			len = 4;
 			break;
 
 		case WID_INT:
-			do {
-				struct wilc_cfg_word *w = &wl->cfg.w[i];
+			while (cfg->w[i].id != WID_NIL && cfg->w[i].id != wid)
+				i++;
 
-				if (w->id == WID_NIL)
-					break;
+			if (cfg->w[i].id == wid)
+				cfg->w[i].val = get_unaligned_le32(&info[4]);
 
-				if (w->id == wid) {
-					w->val = get_unaligned_le32(&info[4]);
-					break;
-				}
-				i++;
-			} while (1);
 			len = 6;
 			break;
 
 		case WID_STR:
-			do {
-				if (wl->cfg.s[i].id == WID_NIL)
-					break;
-
-				if (wl->cfg.s[i].id == wid) {
-					memcpy(wl->cfg.s[i].str, &info[2],
-					       (info[2] + 2));
-					break;
-				}
+			while (cfg->s[i].id != WID_NIL && cfg->s[i].id != wid)
 				i++;
-			} while (1);
+
+			if (cfg->s[i].id == wid)
+				memcpy(cfg->s[i].str, &info[2], info[2] + 2);
+
 			len = 2 + info[2];
 			break;
 
@@ -223,16 +203,12 @@  static void wilc_wlan_parse_info_frame(struct wilc *wl, u8 *info)
 	if (len == 1 && wid == WID_STATUS) {
 		int i = 0;
 
-		do {
-			if (wl->cfg.b[i].id == WID_NIL)
-				break;
-
-			if (wl->cfg.b[i].id == wid) {
-				wl->cfg.b[i].val = info[3];
-				break;
-			}
+		while (wl->cfg.b[i].id != WID_NIL &&
+		       wl->cfg.b[i].id != wid)
 			i++;
-		} while (1);
+
+		if (wl->cfg.b[i].id == wid)
+			wl->cfg.b[i].val = info[3];
 	}
 }
 
@@ -292,63 +268,45 @@  int wilc_wlan_cfg_get_val(struct wilc *wl, u16 wid, u8 *buffer,
 {
 	u8 type = FIELD_GET(WILC_WID_TYPE, wid);
 	int i, ret = 0;
+	struct wilc_cfg *cfg = &wl->cfg;
 
 	i = 0;
 	if (type == CFG_BYTE_CMD) {
-		do {
-			if (wl->cfg.b[i].id == WID_NIL)
-				break;
-
-			if (wl->cfg.b[i].id == wid) {
-				memcpy(buffer, &wl->cfg.b[i].val, 1);
-				ret = 1;
-				break;
-			}
+		while (cfg->b[i].id != WID_NIL && cfg->b[i].id != wid)
 			i++;
-		} while (1);
+
+		if (cfg->b[i].id == wid) {
+			memcpy(buffer, &cfg->b[i].val, 1);
+			ret = 1;
+		}
 	} else if (type == CFG_HWORD_CMD) {
-		do {
-			if (wl->cfg.hw[i].id == WID_NIL)
-				break;
-
-			if (wl->cfg.hw[i].id == wid) {
-				memcpy(buffer, &wl->cfg.hw[i].val, 2);
-				ret = 2;
-				break;
-			}
+		while (cfg->hw[i].id != WID_NIL && cfg->hw[i].id != wid)
 			i++;
-		} while (1);
+
+		if (cfg->hw[i].id == wid) {
+			memcpy(buffer, &cfg->hw[i].val, 2);
+			ret = 2;
+		}
 	} else if (type == CFG_WORD_CMD) {
-		do {
-			if (wl->cfg.w[i].id == WID_NIL)
-				break;
-
-			if (wl->cfg.w[i].id == wid) {
-				memcpy(buffer, &wl->cfg.w[i].val, 4);
-				ret = 4;
-				break;
-			}
+		while (cfg->w[i].id != WID_NIL && cfg->w[i].id != wid)
 			i++;
-		} while (1);
-	} else if (type == CFG_STR_CMD) {
-		do {
-			u32 id = wl->cfg.s[i].id;
 
-			if (id == WID_NIL)
-				break;
+		if (cfg->w[i].id == wid) {
+			memcpy(buffer, &cfg->w[i].val, 4);
+			ret = 4;
+		}
+	} else if (type == CFG_STR_CMD) {
+		while (cfg->s[i].id != WID_NIL && cfg->s[i].id != wid)
+			i++;
 
-			if (id == wid) {
-				u16 size = get_unaligned_le16(wl->cfg.s[i].str);
+		if (cfg->s[i].id == wid) {
+			u16 size = get_unaligned_le16(cfg->s[i].str);
 
-				if (buffer_size >= size) {
-					memcpy(buffer, &wl->cfg.s[i].str[2],
-					       size);
-					ret = size;
-				}
-				break;
+			if (buffer_size >= size) {
+				memcpy(buffer, &cfg->s[i].str[2], size);
+				ret = size;
 			}
-			i++;
-		} while (1);
+		}
 	}
 	return ret;
 }