On Wed, 2010-02-17 at 20:04 +0100, okias wrote:
> ok, sorry I tested it on x86_64. Here is correct patch:
I'm sorry, but it's not a cleanup. Initializing reg outside the block
gives a wrong impression that reg may be used outside the block. Code
is safer is it's more readable, and it's more readable if the logic is
not spread around, but kept in one place.
Unnecessary initialization can prevent gcc from issuing warnings about
using uninitialized variables when the code changes. Such warnings may
be useful to find real bugs. I can give you a realistic example, but it
would be off-topic here.
A real cleanup would be to give reg the block scope and to refactor the
code where reg is written to the hardware:
--- a/drivers/net/wireless/rtl818x/rtl8187_dev.c
+++ b/drivers/net/wireless/rtl818x/rtl8187_dev.c
@@ -1163,9 +1163,10 @@ static void rtl8187_bss_info_changed(struct ieee80211_hw *dev,
{
struct rtl8187_priv *priv = dev->priv;
int i;
- u8 reg;
if (changed & BSS_CHANGED_BSSID) {
+ u8 reg;
+
mutex_lock(&priv->conf_mutex);
for (i = 0; i < ETH_ALEN; i++)
rtl818x_iowrite8(priv, &priv->map->BSSID[i],
@@ -1176,13 +1177,12 @@ static void rtl8187_bss_info_changed(struct ieee80211_hw *dev,
else
reg = 0;
- if (is_valid_ether_addr(info->bssid)) {
+ if (is_valid_ether_addr(info->bssid))
reg |= RTL818X_MSR_INFRA;
- rtl818x_iowrite8(priv, &priv->map->MSR, reg);
- } else {
+ else
reg |= RTL818X_MSR_NO_LINK;
- rtl818x_iowrite8(priv, &priv->map->MSR, reg);
- }
+
+ rtl818x_iowrite8(priv, &priv->map->MSR, reg);
mutex_unlock(&priv->conf_mutex);
}
But I would never bother with that simply because wireless developers
have more important things to do.
I respectfully suggest that you do your exercises in C programming
elsewhere.
--
Regards,
Pavel Roskin
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
