Hi, On Mon, 2006-11-06 at 10:34 +0000, Al Viro wrote: > On Mon, Nov 06, 2006 at 10:32:43AM +0000, Al Viro wrote: > > On Mon, Nov 06, 2006 at 10:31:02AM +0000, Steven Whitehouse wrote: > > > + opt->opt_optl = dn_htons((__u16)*ptr++); > > > > Lose that cast; it's only confusing the things... > > > > > + memcpy(opt->opt_data, ptr, dn_ntohs(opt->opt_optl)); > > > + skb_pull(skb, dn_ntohs(opt->opt_optl) + 1); > > > > ... and I'd actually do > > > > u16 len = *ptr++; /* yes, it's 8bit on the wire */ > > opt->opt_optl = dn_htons(len); > > BUG_ON(len > 16); /* we've checked the contents earlier */ > > memcpy(opt->opt_data, ptr, len); > > skb_pull(skb, len + 1); > > BTW, why the hell do we keep ->opt_optl __le16 internally? If we ever > pass it to userland, fine, but let's convert to __le16 *then*...
Really the only thing that we do with this data is verify it and pass to userland. It does mean that getsockopt() is simpler for just being able to use copy_to_user() with a ptr & len depending on which of the structures the user has requested rather than having to convert each field of each structure for example. I'm not sure its worth changing it now, for saving just one byte per socket in this case, Steve. - 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
