Skip to contentSkip to conversation

block/rnbd: Fix an IS_ERR() vs NULL check in find_or_create_sess()

PATCH In review Dan Carpenter 3 participants 6 messages May 19, 2020 – May 22, 2020

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 block/rnbd: Fix an IS_ERR() vs NULL check in find_or_create_sess()

The alloc_sess() function returns error pointers, it never returns NULL.

Fixes f7a7a5c228d4 ("block/rnbd: client: main functionality")
---
 drivers/block/rnbd/rnbd-clt.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/block/rnbd/rnbd-clt.c +4 -5
index 55bff3b1be71..a033247281da 100644
@@ -923,13 +923,12 @@ rnbd_clt_session *find_or_create_sess(const char *sessname, bool *first)
 	sess = __find_and_get_sess(sessname);
 	if (!sess) {
 		sess = alloc_sess(sessname);
-		if (sess) {
-			list_add(&sess->list, &sess_list);
-			*first = true;
-		} else {
+		if (IS_ERR(sess)) {
 			mutex_unlock(&sess_lock);
-			return ERR_PTR(-ENOMEM);
+			return sess;
 		}
+		list_add(&sess->list, &sess_list);
+		*first = true;
 	} else
 		*first = false;
 	mutex_unlock(&sess_lock);
Signature · 1 line
2.26.2
Reviewed-by on v1 1/1
#1 lore raw
Quoted 5 lines · Dan Carpenter

The alloc_sess() function returns error pointers, it never returns NULL.

Fixes f7a7a5c228d4 ("block/rnbd: client: main functionality")

Thanks Dan,

Quoted 29 lines
---
 drivers/block/rnbd/rnbd-clt.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/block/rnbd/rnbd-clt.c +4 -6
index 55bff3b1be71..a033247281da 100644
@@ -923,13 +923,12 @@ rnbd_clt_session *find_or_create_sess(const char *sessname, bool *first)
        sess = __find_and_get_sess(sessname);
        if (!sess) {
                sess = alloc_sess(sessname);
-               if (sess) {
-                       list_add(&sess->list, &sess_list);
-                       *first = true;
-               } else {
+               if (IS_ERR(sess)) {
                        mutex_unlock(&sess_lock);
-                       return ERR_PTR(-ENOMEM);
+                       return sess;
                }
+               list_add(&sess->list, &sess_list);
+               *first = true;
        } else
                *first = false;
        mutex_unlock(&sess_lock);
--

2.26.2

#3 lore raw
Quoted 6 lines · Jinpu Wang
Quoted 5 lines · Jinpu Wang
Quoted 3 lines · Dan Carpenter

The alloc_sess() function returns error pointers, it never returns NULL.

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Fixes f7a7a5c228d4 ("block/rnbd: client: main functionality")

Thanks Dan,

Hi Jason,

Could you also queue this fix for for-next?

Uhh.. Yes OK, but if it doesn't get cc'd to linux-rdma I won't see it..

Please let me know if there are others

Thanks,
Jason

#4 lore raw
Quoted 9 lines · Jason Gunthorpe
Quoted 8 lines · Jinpu Wang
Quoted 7 lines · Jinpu Wang
Quoted 5 lines · Dan Carpenter

The alloc_sess() function returns error pointers, it never returns NULL.

Fixes f7a7a5c228d4 ("block/rnbd: client: main functionality")
Quoted 9 lines
Quoted 6 lines
Quoted 2 lines

Thanks Dan,

Hi Jason,

Could you also queue this fix for for-next?

Uhh.. Yes OK, but if it doesn't get cc'd to linux-rdma I won't see it..

I suspect that we should update MAINTAINERS so that ./scripts/get_maintainer.pl gives the right lists. Proabably all drivers/block/rnbd/ patches are supposed to go through you?

regards,
dan carpenter

#5 lore raw
Quoted 15 lines · Dan Carpenter
Quoted 14 lines · Jason Gunthorpe
Quoted 13 lines · Jinpu Wang
Quoted 9 lines · Jinpu Wang
Quoted 5 lines · Dan Carpenter

The alloc_sess() function returns error pointers, it never returns NULL.

Fixes f7a7a5c228d4 ("block/rnbd: client: main functionality")

Thanks Dan,

Hi Jason,

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

Could you also queue this fix for for-next?

Uhh.. Yes OK, but if it doesn't get cc'd to linux-rdma I won't see it..

I suspect that we should update MAINTAINERS so that ./scripts/get_maintainer.pl gives the right lists. Proabably all drivers/block/rnbd/ patches are supposed to go through you?

I think Jens will take then once everything is merged, it is just for the next few weeks

Jason