On Fri, 6 Nov 2015, walter harms wrote:
> +1
>
> I like this more since it is much more obvious what is done.
>
> more over we can remove a macro with only 2 users.
It's not to bad because it turns out to be pretty concise. But I don't
agree about the 2 users. There are 2 users for this specific definition,
but the same concept is used in 37 places, and having an inlined solution
in one place and a macro based solution in another place is not going to
be helpful. So the decision should be made in terms of all 37 users.
julia
>
> re,
> wh
>
>
> Am 06.11.2015 11:57, schrieb Julia Lawall:
> >>> Would it be preferable to remove the macro entirely and inline the for
> >>> loop header?
> >>
> >> Could you show me an example of how this would look exactly?
> >
> > One possible solution is below. I moved the initialization of the nh
> > pointer inside the loop to reduce the size of the loop header. One could
> > also inline fi->fib_nh[nhsel] where it occurs, but it seemed that that
> > would make quite long expressions.
> >
> > julia
> >
> > diff --git a/net/decnet/dn_table.c b/net/decnet/dn_table.c
> > index 1540b50..509ae82 100644
> > --- a/net/decnet/dn_table.c
> > +++ b/net/decnet/dn_table.c
> > @@ -60,11 +60,6 @@ struct dn_hash
> >
> > #define dz_key_0(key) ((key).datum = 0)
> >
> > -#define for_nexthops(fi) { int nhsel; const struct dn_fib_nh *nh;\
> > - for(nhsel = 0, nh = (fi)->fib_nh; nhsel < (fi)->fib_nhs; nh++, nhsel++)
> > -
> > -#define endfor_nexthops(fi) }
> > -
> > #define DN_MAX_DIVISOR 1024
> > #define DN_S_ZOMBIE 1
> > #define DN_S_ACCESSED 2
> > @@ -227,7 +222,7 @@ static struct dn_zone *dn_new_zone(struct dn_hash *table, int z)
> > static int dn_fib_nh_match(struct rtmsg *r, struct nlmsghdr *nlh, struct nlattr *attrs[], struct dn_fib_info *fi)
> > {
> > struct rtnexthop *nhp;
> > - int nhlen;
> > + int nhlen, nhsel;
> >
> > if (attrs[RTA_PRIORITY] &&
> > nla_get_u32(attrs[RTA_PRIORITY]) != fi->fib_priority)
> > @@ -246,8 +241,9 @@ static int dn_fib_nh_match(struct rtmsg *r, struct nlmsghdr *nlh, struct nlattr
> > nhp = nla_data(attrs[RTA_MULTIPATH]);
> > nhlen = nla_len(attrs[RTA_MULTIPATH]);
> >
> > - for_nexthops(fi) {
> > + for (nhsel = 0; nhsel < fi->fib_nhs; nhsel++) {
> > int attrlen = nhlen - sizeof(struct rtnexthop);
> > + const struct dn_fib_nh *nh = &fi->fib_nh[nhsel];
> > __le16 gw;
> >
> > if (attrlen < 0 || (nhlen -= nhp->rtnh_len) < 0)
> > @@ -264,7 +260,7 @@ static int dn_fib_nh_match(struct rtmsg *r, struct nlmsghdr *nlh, struct nlattr
> > return 1;
> > }
> > nhp = RTNH_NEXT(nhp);
> > - } endfor_nexthops(fi);
> > + }
> >
> > return 0;
> > }
> > @@ -345,11 +341,13 @@ static int dn_fib_dump_info(struct sk_buff *skb, u32 portid, u32 seq, int event,
> > if (fi->fib_nhs > 1) {
> > struct rtnexthop *nhp;
> > struct nlattr *mp_head;
> > + int nhsel;
> >
> > if (!(mp_head = nla_nest_start(skb, RTA_MULTIPATH)))
> > goto errout;
> >
> > - for_nexthops(fi) {
> > + for (nhsel = 0; nhsel < fi->fib_nhs; nhsel++) {
> > + const struct dn_fib_nh *nh = &fi->fib_nh[nhsel];
> > if (!(nhp = nla_reserve_nohdr(skb, sizeof(*nhp))))
> > goto errout;
> >
> > @@ -362,7 +360,7 @@ static int dn_fib_dump_info(struct sk_buff *skb, u32 portid, u32 seq, int event,
> > goto errout;
> >
> > nhp->rtnh_len = skb_tail_pointer(skb) - (unsigned char *)nhp;
> > - } endfor_nexthops(fi);
> > + }
> >
> > nla_nest_end(skb, mp_head);
> > }
> > --
> > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
