On 09/01/2010 02:14 PM, Jeff Layton wrote:
On Wed, 1 Sep 2010 12:00:06 -0700
Ben Greear<greearb@xxxxxxxxxxxxxxx> wrote:
+ struct sockaddr *srcaddr;
+ srcaddr = (struct sockaddr *)(&tcon->ses->server->srcaddr);
^^^ nit: parens not needed here
Ok, will fix in next patch.
seq_printf(s, ",unc=%s", tcon->treeName);
if (tcon->ses->userName)
@@ -374,6 +377,19 @@ cifs_show_options(struct seq_file *s, struct vfsmount *m)
if (tcon->ses->domainName)
seq_printf(s, ",domain=%s", tcon->ses->domainName);
+ if (srcaddr->sa_family != AF_UNSPEC) {
+ struct sockaddr_in *saddr4;
+ struct sockaddr_in6 *saddr6;
+ saddr4 = (struct sockaddr_in *)srcaddr;
+ saddr6 = (struct sockaddr_in6 *)srcaddr;
+ if (saddr6->sin6_family == AF_INET6)
+ seq_printf(s, ",srcaddr=%pI6c",
+ &saddr6->sin6_addr);
+ else
+ seq_printf(s, ",srcaddr=%pI4",
+ &saddr4->sin_addr.s_addr);
^^^ It's unlikely to occur, but maybe better to make
this a switch() and have a default: case that doesn't
prints the address as "(unknown)" or something? It's
usually better to code defensively for this sort of
stuff and printing a garbage address may be confusing
for users.
I'll work on that.
+/** Returns true if srcaddr isn't specified and rhs isn't
+ * specified, or if srcaddr is specified and
+ * matches the IP address of the rhs argument.
+ */
+static bool
+srcip_matches(struct sockaddr *srcaddr, struct sockaddr *rhs)
+{
+ switch (srcaddr->sa_family) {
+ case AF_UNSPEC:
+ return (rhs->sa_family == AF_UNSPEC);
+ case AF_INET: {
+ struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr;
+ struct sockaddr_in *vaddr4 = (struct sockaddr_in *)rhs;
+ return (saddr4->sin_addr.s_addr == vaddr4->sin_addr.s_addr);
+ }
+ case AF_INET6: {
+ struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr;
+ struct sockaddr_in6 *vaddr6 = (struct sockaddr_in6 *)&rhs;
+ return ipv6_addr_equal(&saddr6->sin6_addr,&vaddr6->sin6_addr);
+ }
^^^^^
These curly braces aren't needed.
It won't compile without them. I'd have to declare those variables before the
switch to do away with the parens, and I prefer it as I wrote it. I'll change
it if you all prefer it otherwise, however.
+ default:
+ WARN_ON(1);
Again, I'm not a huge fan of the cERROR and cFYI macros, but they are
our "standard". This would probably be best as a cERROR macro. You
should probably also have it print the value of srcaddr->sa_family as
that may be useful for debugging.
+ return false; /* don't expect to be here */
+ }
+}
Does the above generate a compiler warning about reaching end of a
non-void function? Either way, it's less clear. I'd change the default
to just fall through and move the "return false" outside the switch.
No warning, it always returns something since the default case catches
all others. If I did put the return at the end, then the compiler wouldn't
catch a case where I forgot to return from one of the case statements,
but it's not overly complex code, so I don't care so much either way.
Plz let me know if you still want it at the end.
I also think the WARN_ON is valid, because it can only be a coding bug
that hits that state, and I'd like it to be as loud as possible while
still allowing the user to continue. There are automated tools that
catch WARN_ON output and post to kernel bug trackers, for instance.
If you still want a cERROR, I can do that..but I prefer to not waste
the space.
Thanks,
Ben
--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc http://www.candelatech.com
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
