IB/rxe: Warn when objects are invalid

Commit Message

Yuval Shaia Dec. 6, 2018, 11:09 a.m. UTC

Since callers never validates return value let's make sure objects are
valid here and do not return NULL.

Signed-off-by: Yuval Shaia <[e-mail]>
---
 drivers/infiniband/sw/rxe/rxe_verbs.h | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

Comments

Bart Van Assche Dec. 6, 2018, 5:44 p.m. UTC | #1

On Thu, 2018-12-06 at 13:09 +0200, Yuval Shaia wrote:
> Since callers never validates return value let's make sure objects are
> valid here and do not return NULL.

This is not sufficient as motivation to make this change. A better motivation
would e.g. be that you have audited all callers and verified that none of the
callers passes a NULL pointer. Another possible motivation could be that you
have tested these code paths extensively and that none of the removed warning
statements has been triggered.

Bart.

Yuval Shaia Dec. 10, 2018, 9:46 p.m. UTC | #2

On Thu, Dec 06, 2018 at 09:44:41AM -0800, Bart Van Assche wrote:
> On Thu, 2018-12-06 at 13:09 +0200, Yuval Shaia wrote:
> > Since callers never validates return value let's make sure objects are
> > valid here and do not return NULL.
> 
> This is not sufficient as motivation to make this change. A better motivation
> would e.g. be that you have audited all callers and verified that none of the
> callers passes a NULL pointer. Another possible motivation could be that you
> have tested these code paths extensively and that none of the removed warning
> statements has been triggered.

Hmm...that i couldn't say.

What i could say though is that i covered all places that uses these
inlines and none of them checks the return value.
So, with the current code we will hit NULL pointer exception when the
return value will be used (e.x rxe object in the case of to_rdev) where the
source of error is that dev is NULL.

I guess that this patch does not bring such improvement, isn't it.

Let's drop it then.

> 
> Bart.

Patch

diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
index 831381b7788d..540e79aebd6b 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.h
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
@@ -423,47 +423,56 @@  static inline void rxe_counter_inc(struct rxe_dev *rxe, enum rxe_counters cnt)
 
 static inline struct rxe_dev *to_rdev(struct ib_device *dev)
 {
-	return dev ? container_of(dev, struct rxe_dev, ib_dev) : NULL;
+	WARN_ONCE(!dev, "Invalid ib_device\n");
+	return container_of(dev, struct rxe_dev, ib_dev);
 }
 
 static inline struct rxe_ucontext *to_ruc(struct ib_ucontext *uc)
 {
-	return uc ? container_of(uc, struct rxe_ucontext, ibuc) : NULL;
+	WARN_ONCE(!uc, "Invalid ib_ucontext\n");
+	return container_of(uc, struct rxe_ucontext, ibuc);
 }
 
 static inline struct rxe_pd *to_rpd(struct ib_pd *pd)
 {
-	return pd ? container_of(pd, struct rxe_pd, ibpd) : NULL;
+	WARN_ONCE(!pd, "Invalid ib_PD\n");
+	return container_of(pd, struct rxe_pd, ibpd);
 }
 
 static inline struct rxe_ah *to_rah(struct ib_ah *ah)
 {
-	return ah ? container_of(ah, struct rxe_ah, ibah) : NULL;
+	WARN_ONCE(!ah, "Invalid ib_AH\n");
+	return container_of(ah, struct rxe_ah, ibah);
 }
 
 static inline struct rxe_srq *to_rsrq(struct ib_srq *srq)
 {
-	return srq ? container_of(srq, struct rxe_srq, ibsrq) : NULL;
+	WARN_ONCE(!srq, "Invalid ib_SRQ\n");
+	return container_of(srq, struct rxe_srq, ibsrq);
 }
 
 static inline struct rxe_qp *to_rqp(struct ib_qp *qp)
 {
-	return qp ? container_of(qp, struct rxe_qp, ibqp) : NULL;
+	WARN_ONCE(!qp, "Invalid ib_QP\n");
+	return container_of(qp, struct rxe_qp, ibqp);
 }
 
 static inline struct rxe_cq *to_rcq(struct ib_cq *cq)
 {
-	return cq ? container_of(cq, struct rxe_cq, ibcq) : NULL;
+	WARN_ONCE(!cq, "Invalid ib_CQ\n");
+	return container_of(cq, struct rxe_cq, ibcq);
 }
 
 static inline struct rxe_mem *to_rmr(struct ib_mr *mr)
 {
-	return mr ? container_of(mr, struct rxe_mem, ibmr) : NULL;
+	WARN_ONCE(!mr, "Invalid ib_MR\n");
+	return container_of(mr, struct rxe_mem, ibmr);
 }
 
 static inline struct rxe_mem *to_rmw(struct ib_mw *mw)
 {
-	return mw ? container_of(mw, struct rxe_mem, ibmw) : NULL;
+	WARN_ONCE(!mw, "Invalid ib_MW\n");
+	return container_of(mw, struct rxe_mem, ibmw);
 }
 
 int rxe_register_device(struct rxe_dev *rxe);