From: Ajay Singh <ajay.kathat@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx/ Signed-off-by: Ajay Singh <ajay.kathat@xxxxxxxxxxxxx> --- 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 --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; } -- 2.24.0
