Re: [PATCH 04/10] block/rnbd-srv: no need to check sess_dev

[Jinpu Wang <jinpu.wang@xxxxxxxxx>]

On Wed, May 24, 2023 at 3:35 AM Guoqing Jiang <guoqing.jiang@xxxxxxxxx> wrote:
>
>
>
> On 5/23/23 17:27, Jinpu Wang wrote:
> > On Tue, May 23, 2023 at 9:53 AM Guoqing Jiang <guoqing.jiang@xxxxxxxxx> wrote:
> >> Check ret is enough since if sess_dev is NULL which also
> >> implies ret should be 0.
> >>
> >> Signed-off-by: Guoqing Jiang <guoqing.jiang@xxxxxxxxx>
> >> ---
> >>   drivers/block/rnbd/rnbd-srv.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> >> index e9ef6bd4b50c..c4122e65b36a 100644
> >> --- a/drivers/block/rnbd/rnbd-srv.c
> >> +++ b/drivers/block/rnbd/rnbd-srv.c
> >> @@ -96,7 +96,7 @@ rnbd_get_sess_dev(int dev_id, struct rnbd_srv_session *srv_sess)
> >>                  ret = kref_get_unless_zero(&sess_dev->kref);
> >>          rcu_read_unlock();
> >>
> >> -       if (!sess_dev || !ret)
> >> +       if (!ret)
> >>                  return ERR_PTR(-ENXIO);
> >>
> >>          return sess_dev;
> >> --
> >> 2.35.3
> > This looks wrong, if you drop the check for !sess_dev, you have to
> > check it later with IS_ERR_OR_NULL when call rnbd_get_sess_dev
>
> How can it return NULL? Pls correct me if I am wrong.
>
> 1. If sess_dev is NULL then ret is 0, so it just returns ERR_PTR(-ENXIO).
> 2. If sess_dev is not NULL, we still rely on the checking of ret.
Ah, you are right.

Acked-by: Jack Wang <jinpu.wang@xxxxxxxxx>
thx!