On Tue, Aug 21, 2012 at 05:29:16PM -0400, Chuck Lever wrote:
>
> On Aug 21, 2012, at 5:24 PM, J. Bruce Fields wrote:
>
> > On Tue, Aug 21, 2012 at 05:02:14PM -0400, Chuck Lever wrote:
> >>
> >> On Aug 21, 2012, at 4:57 PM, J. Bruce Fields wrote:
> >>
> >>> From: "J. Bruce Fields" <bfields@xxxxxxxxxx>
> >>>
> >>> How would this happen?
> >>
> >> If an unsupported address family is used in the rqstp.
> >
> > Right, but that's impossible, isn't it?
>
> The point is to catch this case when someone adds support for new address families. It won't happen in current code.
>
> >>> In any case, it appears this would be returned all the way up to the
> >>> caller of svc_recv(), and it's obvious that none of them are equipped to
> >>> handle it, and not clear what they would want to do with it anyway.
> >>> Let's just drop this and return -EAGAIN.
> >>
> >> EAGAIN is incorrect; the correct error code is EAFNOSUPPORT. If callers are not prepared for this error return, perhaps BUG_ON() would be more appropriate here.
> >
> > Yeah. Actually on a quick check this is the only caller that even
> > checks for this case. So probably the check should go in svc_addr_len.
> > Maybe we should be nice and make it a warning.
>
> I normally find BUG pretty harsh, but in this case it's true a software error, and as you say, should be "impossible": thus it should be a BUG. The code should stop before the zero length is used.
Eh, OK, I guess I can live with that.
commit 751f877b10f8ce0d12b40d2a102f3b42b26dc08d
Author: J. Bruce Fields <bfields@xxxxxxxxxx>
Date: Tue Aug 21 17:22:11 2012 -0400
svcrpc: don't bother checking bad svc_addr_len result
None of the callers should see an unsupported address family (only one
of them even bothers to check for that case), so just check for the
buggy case in svc_addr_len and don't bother elsewhere.
Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index b3f64b1..4f3a6ab 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -166,8 +166,7 @@ static inline size_t svc_addr_len(const struct sockaddr *sa)
case AF_INET6:
return sizeof(struct sockaddr_in6);
}
-
- return 0;
+ BUG();
}
static inline unsigned short svc_xprt_local_port(const struct svc_xprt *xprt)
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 998aa8c..13b005c 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -601,8 +601,6 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
return -EAGAIN;
}
len = svc_addr_len(svc_addr(rqstp));
- if (len == 0)
- return -EAFNOSUPPORT;
rqstp->rq_addrlen = len;
if (skb->tstamp.tv64 == 0) {
skb->tstamp = ktime_get_real();
--
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
