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

Guoqing Jiang <guoqing.jiang@xxxxxxxxx> · Wed, 24 May 2023 09:35:05 +0800

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.

But I can drop it as well if you think it is obscure.

Thanks,
Guoqing






