On Wed, 2009-02-11 at 11:24 -0500, Chuck Lever wrote:
> > + /* Compare scope_id. This may only matter for link local
> > + * addresses, but it is cheap, and should be harmless, to just
> > + * compare it anyway.
> > + */
> > + if (saddr1->sin6_scope_id != saddr2->sin6_scope_id)
> > + return 0;
>
> I've been thinking about this a bit over the past couple of days. I'm
> not yet sure we need this comparison here, but I also don't think it's
> harmless to just leave it in.
As I understand it, it is allowed for a host to see two identical link
local addresses which are on different interfaces but refer to different
servers, so it would need to be in for the case of link local addresses.
But in other cases it is not clear and unfortunately RFC 3493 if not
very helpful on the subject. But non link-local addresses should be
unique so at least it would be safe to not test it in those cases.
> If you really want to include this particular comparison, it would be
> better IMO if you check the equivalence of the scope ID fields only if
> the addresses are link-local.
This seems like the safest course of action.
With these changes, the nfs_sockaddr_cmp_ip6 function is starting to
become significant. I was trying to avoid it, but should it be within a
#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) ...
#endif
block? Someone wanting a minimal (maybe embedded) kernel might object to
the "bloat".
> > + return (saddr1->sin6_port == saddr2->sin6_port);
>
> The parentheses are not needed here.
I know, but are they frowned upon? AFAIK there is no shortage of
parentheses and they often make things clearer even when redundant.
> Kernel C coding style frowns on the space between the function name
> and the argument list.
Is there a style guide or is it more of an osmosis thing? Stylistically,
there are lots of things I'd do differently (4 character indents for a
start) but I try and conform with existing practice. Reading the
existing code isn't always a good guide though.
--
Ian Dall <ian@xxxxxxxxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
