Re: lots of code could be simplified by using ARRAY_SIZE()

[Tim Schmielau]

On Sat, 16 Dec 2006, Robert P. J. Day wrote:
> On Fri, 15 Dec 2006, Tim Schmielau wrote:
> >
> > It might be interesting to grep for anything that divides two
> > sizeofs and eyeball the result for possible mistakes. This would
> > provide some real benefit beyond the cosmetical changes.
> 
> i did that a while ago and it's amazing the variation that you find
> beyond the obvious:
> 
> $ grep -Er "sizeof.*/.*sizeof" . | less
> 
> ...
> ./net/key/af_key.c:     sa->sadb_sa_len = sizeof(struct sadb_sa)/sizeof(uint64_t);
> ./net/xfrm/xfrm_policy.c:       int len = sizeof(struct xfrm_selector) / sizeof(u32);
> ./net/core/flow.c:      const int n_elem = sizeof(struct flowi) / sizeof(flow_compare_t);
> ./net/ipv4/netfilter/arp_tables.c:      for (i = 0; i < sizeof(*arp)/sizeof(__u32); i++)
> ./net/ipv4/af_inet.c:#define INETSW_ARRAY_LEN (sizeof(inetsw_array) / sizeof(struct inet_protosw))
> ./drivers/net/wireless/ray_cs.c:        .num_standard   = sizeof(ray_handler)/sizeof(iw_handler),
> 

Of the above, af_inet.c and ray_cs.c seem to be good candidates for 
ARRAY_SIZE. You might even remove the INETSW_ARRAY_LEN #define in 
af_inet.c altogether, since ARRAY_SIZE(inetsw_array) is quite readable.

xfrm_policy.c, flow.c and arp_tables.c seem to be reasonably readable 
trickery that can be left as-is.

>From a first glance, af_key.c is ok but might profit from a comment 
in include/linux/pfkeyv2.h saying that sadb_msg_len is measured in 64-bit 
words per RFC 2367. Though documenting the structs in pfkeyv2.h would be
quite a bit different from what you initially intended...

So, if you have some time to spend on this, manual inspection would 
probably be the most useful thing, since any automatic sed tricks will 
only replace what a human ready would easily understand as well.

If you manually generate cleanup patches, it would be very good to check 
that compilation with allyesconfig generates identical code before and 
after before feeding them through the respective maintainers.

If you want to find genuine bugs by this, it might be more worthwhile to 
start with drivers/, as davem is just too clever to make any mistakes. 

Not that I want to make you spend you time on this. It's just beause you 
asked.

Tim
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/