| Message ID | 20220719090948.612921-1-zys.zljxml@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | RDMA/cxgb4: Cleanup unused assignments | expand |
On Tue, Jul 19, 2022 at 05:09:48PM +0800, zys.zljxml@gmail.com wrote: > From: Yushan Zhou <katrinzhou@tencent.com> > > The variable err is reassigned before the assigned value works. > Cleanup unused assignments reported by Coverity. > > Addresses-Coverity: ("UNUSED_VALUE") > Signed-off-by: Yushan Zhou <katrinzhou@tencent.com> > --- > drivers/infiniband/hw/cxgb4/cm.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c > index c16017f6e8db..3462fe991f93 100644 > --- a/drivers/infiniband/hw/cxgb4/cm.c > +++ b/drivers/infiniband/hw/cxgb4/cm.c > @@ -1590,7 +1590,6 @@ static int process_mpa_reply(struct c4iw_ep *ep, struct sk_buff *skb) > insuff_ird = 1; > } > if (insuff_ird) { > - err = -ENOMEM; > ep->ird = resp_ord; > ep->ord = resp_ird; > } > @@ -1655,7 +1654,7 @@ static int process_mpa_reply(struct c4iw_ep *ep, struct sk_buff *skb) > attrs.ecode = MPA_NOMATCH_RTR; > attrs.next_state = C4IW_QP_STATE_TERMINATE; > attrs.send_term = 1; > - err = c4iw_modify_qp(ep->com.qp->rhp, ep->com.qp, > + c4iw_modify_qp(ep->com.qp->rhp, ep->com.qp, > C4IW_QP_ATTR_NEXT_STATE, &attrs, 1); > err = -ENOMEM; I would prefer do not overwrite errors returned from the functions unless it is really necessary. Can anyone from chelsio help here? Thanks > disconnect = 1; > @@ -1674,7 +1673,7 @@ static int process_mpa_reply(struct c4iw_ep *ep, struct sk_buff *skb) > attrs.ecode = MPA_INSUFF_IRD; > attrs.next_state = C4IW_QP_STATE_TERMINATE; > attrs.send_term = 1; > - err = c4iw_modify_qp(ep->com.qp->rhp, ep->com.qp, > + c4iw_modify_qp(ep->com.qp->rhp, ep->com.qp, > C4IW_QP_ATTR_NEXT_STATE, &attrs, 1); > err = -ENOMEM; > disconnect = 1; > -- > 2.27.0 >
On Thu, Jul 21, 2022 at 3:18 PM Leon Romanovsky <leon@kernel.org> wrote: > > On Tue, Jul 19, 2022 at 05:09:48PM +0800, zys.zljxml@gmail.com wrote: > > From: Yushan Zhou <katrinzhou@tencent.com> > > > > The variable err is reassigned before the assigned value works. > > Cleanup unused assignments reported by Coverity. > > > > Addresses-Coverity: ("UNUSED_VALUE") > > Signed-off-by: Yushan Zhou <katrinzhou@tencent.com> > > --- > > drivers/infiniband/hw/cxgb4/cm.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c > > index c16017f6e8db..3462fe991f93 100644 > > --- a/drivers/infiniband/hw/cxgb4/cm.c > > +++ b/drivers/infiniband/hw/cxgb4/cm.c > > @@ -1590,7 +1590,6 @@ static int process_mpa_reply(struct c4iw_ep *ep, struct sk_buff *skb) > > insuff_ird = 1; > > } > > if (insuff_ird) { > > - err = -ENOMEM; > > ep->ird = resp_ord; > > ep->ord = resp_ird; > > } > > @@ -1655,7 +1654,7 @@ static int process_mpa_reply(struct c4iw_ep *ep, struct sk_buff *skb) > > attrs.ecode = MPA_NOMATCH_RTR; > > attrs.next_state = C4IW_QP_STATE_TERMINATE; > > attrs.send_term = 1; > > - err = c4iw_modify_qp(ep->com.qp->rhp, ep->com.qp, > > + c4iw_modify_qp(ep->com.qp->rhp, ep->com.qp, > > C4IW_QP_ATTR_NEXT_STATE, &attrs, 1); > > err = -ENOMEM; > > I would prefer do not overwrite errors returned from the functions > unless it is really necessary. > > Can anyone from chelsio help here? > > Thanks > > > disconnect = 1; > > @@ -1674,7 +1673,7 @@ static int process_mpa_reply(struct c4iw_ep *ep, struct sk_buff *skb) > > attrs.ecode = MPA_INSUFF_IRD; > > attrs.next_state = C4IW_QP_STATE_TERMINATE; > > attrs.send_term = 1; > > - err = c4iw_modify_qp(ep->com.qp->rhp, ep->com.qp, > > + c4iw_modify_qp(ep->com.qp->rhp, ep->com.qp, > > C4IW_QP_ATTR_NEXT_STATE, &attrs, 1); > > err = -ENOMEM; > > disconnect = 1; > > -- > > 2.27.0 > > The issue is that, in the original code, there were 2 subsequent assignments to the `err` variable. I assume this is not expected. (I tried to keep the exact same behavior as original code in this patch) Should we keep the first assignment (c4iw_modify_qp), or the second assignment (-ENOMEM)? I'd really appreciate some help here. Best Regards, Katrin
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c index c16017f6e8db..3462fe991f93 100644 --- a/drivers/infiniband/hw/cxgb4/cm.c +++ b/drivers/infiniband/hw/cxgb4/cm.c @@ -1590,7 +1590,6 @@ static int process_mpa_reply(struct c4iw_ep *ep, struct sk_buff *skb) insuff_ird = 1; } if (insuff_ird) { - err = -ENOMEM; ep->ird = resp_ord; ep->ord = resp_ird; } @@ -1655,7 +1654,7 @@ static int process_mpa_reply(struct c4iw_ep *ep, struct sk_buff *skb) attrs.ecode = MPA_NOMATCH_RTR; attrs.next_state = C4IW_QP_STATE_TERMINATE; attrs.send_term = 1; - err = c4iw_modify_qp(ep->com.qp->rhp, ep->com.qp, + c4iw_modify_qp(ep->com.qp->rhp, ep->com.qp, C4IW_QP_ATTR_NEXT_STATE, &attrs, 1); err = -ENOMEM; disconnect = 1; @@ -1674,7 +1673,7 @@ static int process_mpa_reply(struct c4iw_ep *ep, struct sk_buff *skb) attrs.ecode = MPA_INSUFF_IRD; attrs.next_state = C4IW_QP_STATE_TERMINATE; attrs.send_term = 1; - err = c4iw_modify_qp(ep->com.qp->rhp, ep->com.qp, + c4iw_modify_qp(ep->com.qp->rhp, ep->com.qp, C4IW_QP_ATTR_NEXT_STATE, &attrs, 1); err = -ENOMEM; disconnect = 1;