On Thursday 03 January 2008 6:05:18 pm David Miller wrote: > From: Paul Moore <[EMAIL PROTECTED]> > Date: Thu, 3 Jan 2008 16:20:06 -0500 > > > On Thursday 03 January 2008 4:13:12 pm Jarek Poplawski wrote: > > > On Thu, Jan 03, 2008 at 11:15:34AM -0500, Paul Moore wrote: > > > ... > > > > > > > While I'm at it, is there some reason for this #define in > > > > __skb_clone()? > > > > > > > > #define C(x) n->x = skb->x > > > > > > > > ... it seems kinda silly to me and I tend to think the code > > > > would be better without it. > > > > > > IMHO, if there are a lot of this, it's definitely more readable: > > > easier to check which values are simply copied and which need > > > something more. But, as usual, it's probably a question of taste, > > > and of course without it it would definitely look classier... > > > > For me personally, I would argue the readability bit. > > I definitely think the C() thing is more readable. > > Less typing, less reading...
Well, you're the boss :) I just put the C() macro back in, but I kept the reordering that was suggested to help reduce cacheline bounces since that still makes sense to me. The function now looks like this: static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb) { #define C(x) n->x = skb->x n->next = n->prev = NULL; n->sk = NULL; __copy_skb_header(n, skb); C(len); C(data_len); C(mac_len); n->hdr_len = skb->nohdr ? skb_headroom(skb) : skb->hdr_len; n->cloned = 1; n->nohdr = 0; n->destructor = NULL; C(iif); C(tail); C(end); C(head); C(data); C(truesize); atomic_set(&n->users, 1); atomic_inc(&(skb_shinfo(skb)->dataref)); skb->cloned = 1; return n; #undef C } -- paul moore linux security @ hp -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html