On Feb 11, 2009, at 8:28 PM, Ian Dall wrote:
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".
Define bloat -- can you report object code sizes on a couple of common
hardware platforms? I wouldn't expect this new routine to add more
than a few tens of bytes of instructions.
+ 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.
See below.
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.
They have evolved over time, so not all of the kernel is consistent
with current practices which you can read about here:
Documentation/CodingStyle
I don't see a specific missive about return values, but the code
example in Chapter 6 is to the point. I know that Trond (the NFS
maintainer) prefers no parens on return values.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
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
