Re: [DECNET] Endian bug fixes

[Steven Whitehouse <swhiteho@redhat.com>]

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.