net/ipv4/fib_semantics.c: quiet sparse "shadows" noise
If CONFIG_IP_ROUTE_MULTIPATH is defined the symbol 'nh' is
declared twice in the function fib_sync_down_dev. First as the
loop variable used to iterate the hlist. Then again due to the
change_nexthops macro. This produces a sparse warning and
makes the code harder to understand.
Fix the issue by renaming the first symbol.
Signed-off-by: H Hartley Sweeten <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Alexey Kuznetsov <[email protected]>
Cc: "Pekka Savola (ipv6)" <[email protected]>
Cc: James Morris <[email protected]>
Cc: Hideaki YOSHIFUJI <[email protected]>
Cc: Patrick McHardy <[email protected]>
---
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index ed19aa6..cec97f0 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1065,17 +1065,17 @@ int fib_sync_down_dev(struct net_device *dev, int force)
unsigned int hash = fib_devindex_hashfn(dev->ifindex);
struct hlist_head *head = &fib_info_devhash[hash];
struct hlist_node *node;
- struct fib_nh *nh;
+ struct fib_nh *this_nh;
if (force)
scope = -1;
- hlist_for_each_entry(nh, node, head, nh_hash) {
- struct fib_info *fi = nh->nh_parent;
+ hlist_for_each_entry(this_nh, node, head, nh_hash) {
+ struct fib_info *fi = this_nh->nh_parent;
int dead;
BUG_ON(!fi->fib_nhs);
- if (nh->nh_dev != dev || fi == prev_fi)
+ if (this_nh->nh_dev != dev || fi == prev_fi)
continue;
prev_fi = fi;
dead = 0;
From: H Hartley Sweeten <[email protected]>
Date: Thu, 14 Jan 2010 16:52:41 -0700
> net/ipv4/fib_semantics.c: quiet sparse "shadows" noise
>
> If CONFIG_IP_ROUTE_MULTIPATH is defined the symbol 'nh' is
> declared twice in the function fib_sync_down_dev. First as the
> loop variable used to iterate the hlist. Then again due to the
> change_nexthops macro. This produces a sparse warning and
> makes the code harder to understand.
>
> Fix the issue by renaming the first symbol.
>
> Signed-off-by: H Hartley Sweeten <[email protected]>
I think it's easier to fix this by using a less simple name
in change_nexthops(), so I'll deal with this using the following
patch.
Thanks for reporting this:
ipv4: Use less conflicting local var name in change_nexthops() loop macro.
As noticed by H Hartley Sweeten, since change_nexthops() uses 'nh'
as it's iterator variable, it can conflict with other existing
local vars.
Use "nexthop_nh" to avoid the conflict and make it easier to figure
out where this magic variable comes from.
Signed-off-by: David S. Miller <[email protected]>
---
net/ipv4/fib_semantics.c | 76 ++++++++++++++++++++++++----------------------
1 files changed, 40 insertions(+), 36 deletions(-)
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index ed19aa6..96b2101 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -62,8 +62,8 @@ static DEFINE_SPINLOCK(fib_multipath_lock);
#define for_nexthops(fi) { int nhsel; const struct fib_nh * nh; \
for (nhsel=0, nh = (fi)->fib_nh; nhsel < (fi)->fib_nhs; nh++, nhsel++)
-#define change_nexthops(fi) { int nhsel; struct fib_nh * nh; \
-for (nhsel=0, nh = (struct fib_nh *)((fi)->fib_nh); nhsel < (fi)->fib_nhs; nh++, nhsel++)
+#define change_nexthops(fi) { int nhsel; struct fib_nh *nexthop_nh; \
+for (nhsel=0, nexthop_nh = (struct fib_nh *)((fi)->fib_nh); nhsel < (fi)->fib_nhs; nexthop_nh++, nhsel++)
#else /* CONFIG_IP_ROUTE_MULTIPATH */
@@ -72,7 +72,7 @@ for (nhsel=0, nh = (struct fib_nh *)((fi)->fib_nh); nhsel < (fi)->fib_nhs; nh++,
#define for_nexthops(fi) { int nhsel = 0; const struct fib_nh * nh = (fi)->fib_nh; \
for (nhsel=0; nhsel < 1; nhsel++)
-#define change_nexthops(fi) { int nhsel = 0; struct fib_nh * nh = (struct fib_nh *)((fi)->fib_nh); \
+#define change_nexthops(fi) { int nhsel = 0; struct fib_nh *nexthop_nh = (struct fib_nh *)((fi)->fib_nh); \
for (nhsel=0; nhsel < 1; nhsel++)
#endif /* CONFIG_IP_ROUTE_MULTIPATH */
@@ -145,9 +145,9 @@ void free_fib_info(struct fib_info *fi)
return;
}
change_nexthops(fi) {
- if (nh->nh_dev)
- dev_put(nh->nh_dev);
- nh->nh_dev = NULL;
+ if (nexthop_nh->nh_dev)
+ dev_put(nexthop_nh->nh_dev);
+ nexthop_nh->nh_dev = NULL;
} endfor_nexthops(fi);
fib_info_cnt--;
release_net(fi->fib_net);
@@ -162,9 +162,9 @@ void fib_release_info(struct fib_info *fi)
if (fi->fib_prefsrc)
hlist_del(&fi->fib_lhash);
change_nexthops(fi) {
- if (!nh->nh_dev)
+ if (!nexthop_nh->nh_dev)
continue;
- hlist_del(&nh->nh_hash);
+ hlist_del(&nexthop_nh->nh_hash);
} endfor_nexthops(fi)
fi->fib_dead = 1;
fib_info_put(fi);
@@ -395,19 +395,20 @@ static int fib_get_nhs(struct fib_info *fi, struct rtnexthop *rtnh,
if (!rtnh_ok(rtnh, remaining))
return -EINVAL;
- nh->nh_flags = (cfg->fc_flags & ~0xFF) | rtnh->rtnh_flags;
- nh->nh_oif = rtnh->rtnh_ifindex;
- nh->nh_weight = rtnh->rtnh_hops + 1;
+ nexthop_nh->nh_flags =
+ (cfg->fc_flags & ~0xFF) | rtnh->rtnh_flags;
+ nexthop_nh->nh_oif = rtnh->rtnh_ifindex;
+ nexthop_nh->nh_weight = rtnh->rtnh_hops + 1;
attrlen = rtnh_attrlen(rtnh);
if (attrlen > 0) {
struct nlattr *nla, *attrs = rtnh_attrs(rtnh);
nla = nla_find(attrs, attrlen, RTA_GATEWAY);
- nh->nh_gw = nla ? nla_get_be32(nla) : 0;
+ nexthop_nh->nh_gw = nla ? nla_get_be32(nla) : 0;
#ifdef CONFIG_NET_CLS_ROUTE
nla = nla_find(attrs, attrlen, RTA_FLOW);
- nh->nh_tclassid = nla ? nla_get_u32(nla) : 0;
+ nexthop_nh->nh_tclassid = nla ? nla_get_u32(nla) : 0;
#endif
}
@@ -738,7 +739,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
fi->fib_nhs = nhs;
change_nexthops(fi) {
- nh->nh_parent = fi;
+ nexthop_nh->nh_parent = fi;
} endfor_nexthops(fi)
if (cfg->fc_mx) {
@@ -808,7 +809,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
goto failure;
} else {
change_nexthops(fi) {
- if ((err = fib_check_nh(cfg, fi, nh)) != 0)
+ if ((err = fib_check_nh(cfg, fi, nexthop_nh)) != 0)
goto failure;
} endfor_nexthops(fi)
}
@@ -843,11 +844,11 @@ link_it:
struct hlist_head *head;
unsigned int hash;
- if (!nh->nh_dev)
+ if (!nexthop_nh->nh_dev)
continue;
- hash = fib_devindex_hashfn(nh->nh_dev->ifindex);
+ hash = fib_devindex_hashfn(nexthop_nh->nh_dev->ifindex);
head = &fib_info_devhash[hash];
- hlist_add_head(&nh->nh_hash, head);
+ hlist_add_head(&nexthop_nh->nh_hash, head);
} endfor_nexthops(fi)
spin_unlock_bh(&fib_info_lock);
return fi;
@@ -1080,21 +1081,21 @@ int fib_sync_down_dev(struct net_device *dev, int force)
prev_fi = fi;
dead = 0;
change_nexthops(fi) {
- if (nh->nh_flags&RTNH_F_DEAD)
+ if (nexthop_nh->nh_flags&RTNH_F_DEAD)
dead++;
- else if (nh->nh_dev == dev &&
- nh->nh_scope != scope) {
- nh->nh_flags |= RTNH_F_DEAD;
+ else if (nexthop_nh->nh_dev == dev &&
+ nexthop_nh->nh_scope != scope) {
+ nexthop_nh->nh_flags |= RTNH_F_DEAD;
#ifdef CONFIG_IP_ROUTE_MULTIPATH
spin_lock_bh(&fib_multipath_lock);
- fi->fib_power -= nh->nh_power;
- nh->nh_power = 0;
+ fi->fib_power -= nexthop_nh->nh_power;
+ nexthop_nh->nh_power = 0;
spin_unlock_bh(&fib_multipath_lock);
#endif
dead++;
}
#ifdef CONFIG_IP_ROUTE_MULTIPATH
- if (force > 1 && nh->nh_dev == dev) {
+ if (force > 1 && nexthop_nh->nh_dev == dev) {
dead = fi->fib_nhs;
break;
}
@@ -1144,18 +1145,20 @@ int fib_sync_up(struct net_device *dev)
prev_fi = fi;
alive = 0;
change_nexthops(fi) {
- if (!(nh->nh_flags&RTNH_F_DEAD)) {
+ if (!(nexthop_nh->nh_flags&RTNH_F_DEAD)) {
alive++;
continue;
}
- if (nh->nh_dev == NULL || !(nh->nh_dev->flags&IFF_UP))
+ if (nexthop_nh->nh_dev == NULL ||
+ !(nexthop_nh->nh_dev->flags&IFF_UP))
continue;
- if (nh->nh_dev != dev || !__in_dev_get_rtnl(dev))
+ if (nexthop_nh->nh_dev != dev ||
+ !__in_dev_get_rtnl(dev))
continue;
alive++;
spin_lock_bh(&fib_multipath_lock);
- nh->nh_power = 0;
- nh->nh_flags &= ~RTNH_F_DEAD;
+ nexthop_nh->nh_power = 0;
+ nexthop_nh->nh_flags &= ~RTNH_F_DEAD;
spin_unlock_bh(&fib_multipath_lock);
} endfor_nexthops(fi)
@@ -1182,9 +1185,9 @@ void fib_select_multipath(const struct flowi *flp, struct fib_result *res)
if (fi->fib_power <= 0) {
int power = 0;
change_nexthops(fi) {
- if (!(nh->nh_flags&RTNH_F_DEAD)) {
- power += nh->nh_weight;
- nh->nh_power = nh->nh_weight;
+ if (!(nexthop_nh->nh_flags&RTNH_F_DEAD)) {
+ power += nexthop_nh->nh_weight;
+ nexthop_nh->nh_power = nexthop_nh->nh_weight;
}
} endfor_nexthops(fi);
fi->fib_power = power;
@@ -1204,9 +1207,10 @@ void fib_select_multipath(const struct flowi *flp, struct fib_result *res)
w = jiffies % fi->fib_power;
change_nexthops(fi) {
- if (!(nh->nh_flags&RTNH_F_DEAD) && nh->nh_power) {
- if ((w -= nh->nh_power) <= 0) {
- nh->nh_power--;
+ if (!(nexthop_nh->nh_flags&RTNH_F_DEAD) &&
+ nexthop_nh->nh_power) {
+ if ((w -= nexthop_nh->nh_power) <= 0) {
+ nexthop_nh->nh_power--;
fi->fib_power--;
res->nh_sel = nhsel;
spin_unlock_bh(&fib_multipath_lock);
--
1.6.5
On Friday, January 15, 2010 2:18 AM, David Miller wrote:
> From: H Hartley Sweeten <[email protected]>
> Date: Thu, 14 Jan 2010 16:52:41 -0700
>
>> net/ipv4/fib_semantics.c: quiet sparse "shadows" noise
>>
>> If CONFIG_IP_ROUTE_MULTIPATH is defined the symbol 'nh' is
>> declared twice in the function fib_sync_down_dev. First as the
>> loop variable used to iterate the hlist. Then again due to the
>> change_nexthops macro. This produces a sparse warning and
>> makes the code harder to understand.
>>
>> Fix the issue by renaming the first symbol.
>>
>> Signed-off-by: H Hartley Sweeten <[email protected]>
>
> I think it's easier to fix this by using a less simple name
> in change_nexthops(), so I'll deal with this using the following
> patch.
OK. The patch is larger but it's much clearer this way.
> Thanks for reporting this:
>
> ipv4: Use less conflicting local var name in change_nexthops() loop macro.
>
> As noticed by H Hartley Sweeten, since change_nexthops() uses 'nh'
> as it's iterator variable, it can conflict with other existing
> local vars.
>
> Use "nexthop_nh" to avoid the conflict and make it easier to figure
> out where this magic variable comes from.
>
> Signed-off-by: David S. Miller <[email protected]>
Tested-by: H Hartley Sweeten <[email protected]>