On 11/14/22 10:39 PM, Haris Iqbal wrote:
On Sun, Nov 13, 2022 at 2:08 AM Guoqing Jiang<guoqing.jiang@xxxxxxxxx> wrote:The RDMA_CM_EVENT_CONNECT_REQUEST is quite different to other types, let's checking it separately at the beginning of routine, then we can avoid the identation accordingly. Signed-off-by: Guoqing Jiang<guoqing.jiang@xxxxxxxxx> --- drivers/infiniband/ulp/rtrs/rtrs-srv.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c index 79504aaef0cc..2cc8b423bcaa 100644 --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c @@ -1948,24 +1948,19 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id, static int rtrs_srv_rdma_cm_handler(struct rdma_cm_id *cm_id, struct rdma_cm_event *ev) { - struct rtrs_srv_path *srv_path = NULL; - struct rtrs_path *s = NULL; - - if (ev->event != RDMA_CM_EVENT_CONNECT_REQUEST) { - struct rtrs_con *c = cm_id->context; - - s = c->path; - srv_path = to_srv_path(s); - } + struct rtrs_con *c = cm_id->context; + struct rtrs_path *s = c->path; + struct rtrs_srv_path *srv_path = to_srv_path(s);This isn't correct for the RDMA_CM_EVENT_CONNECT_REQUEST event. At that moment, cm_id->context is still holding a pointer to struct rtrs_srv_ctx. Even though it never gets used for this event here, its not right IMO to dereference in this incorrect manner.
But rtrs_rdma_connect will deference the context again for connect request.
How about we move the check for the RDMA_CM_EVENT_CONNECT_REQUEST event outside (just like you did), but let the pointers be NULL, and only dereference after the if condition for RDMA_CM_EVENT_CONNECT_REQUEST event?
Ok, will do it though it need more lines which I tried to avoid. Thanks, Guoqing
