| Message ID | 20200519161345.GA3910@embeddedor (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RDMA/rtrs: client: Fix function return on success | expand |
On 2020-05-19 09:13, Gustavo A. R. Silva wrote: > The function should return 0 on success, instead of err. > > Addresses-Coverity-ID: 1493753 ("Identical code for different branches") > Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality") > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > --- > drivers/infiniband/ulp/rtrs/rtrs-clt.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c > index 468fdd0d8713c..465515e46bb1a 100644 > --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c > +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c > @@ -1594,7 +1594,8 @@ static int create_con_cq_qp(struct rtrs_clt_con *con) > > if (err) > return err; > - return err; > + > + return 0; > } Why to keep the if-statement? Has the following been considered? diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c index 8dfa56dc32bc..a7f5d55f8542 100644 --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c @@ -1587,8 +1587,6 @@ static int create_con_cq_qp(struct rtrs_clt_con *con) * since destroy_con_cq_qp() must be called. */ - if (err) - return err; return err; } Thanks, Bart.
On Tue, May 19, 2020 at 6:16 PM Bart Van Assche <bvanassche@acm.org> wrote: > > On 2020-05-19 09:13, Gustavo A. R. Silva wrote: > > The function should return 0 on success, instead of err. > > > > Addresses-Coverity-ID: 1493753 ("Identical code for different branches") > > Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality") > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > > --- > > drivers/infiniband/ulp/rtrs/rtrs-clt.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c > > index 468fdd0d8713c..465515e46bb1a 100644 > > --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c > > +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c > > @@ -1594,7 +1594,8 @@ static int create_con_cq_qp(struct rtrs_clt_con *con) > > > > if (err) > > return err; > > - return err; > > + > > + return 0; > > } > > Why to keep the if-statement? Has the following been considered? > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c > index 8dfa56dc32bc..a7f5d55f8542 100644 > --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c > +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c > @@ -1587,8 +1587,6 @@ static int create_con_cq_qp(struct rtrs_clt_con *con) > * since destroy_con_cq_qp() must be called. > */ > > - if (err) > - return err; > return err; > } > > Thanks, > > Bart. This one seems better, thanks Bart and Gustavo, Gustavo, would you like to send a v2 patch as Bart suggested? Thanks,
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c index 468fdd0d8713c..465515e46bb1a 100644 --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c @@ -1594,7 +1594,8 @@ static int create_con_cq_qp(struct rtrs_clt_con *con) if (err) return err; - return err; + + return 0; } static void destroy_con_cq_qp(struct rtrs_clt_con *con)
The function should return 0 on success, instead of err. Addresses-Coverity-ID: 1493753 ("Identical code for different branches") Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality") Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- drivers/infiniband/ulp/rtrs/rtrs-clt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)