Re: [PATCH resent] bcache: Fix exception handling in mca_alloc()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/25/23 5:31 PM, Markus Elfring wrote:
Date: Mon, 20 Mar 2023 13:13:37 +0100

The label “err” was used to jump to another pointer check despite of
the detail in the implementation of the function “mca_alloc”
that it was determined already that a corresponding variable contained
a null pointer because of a failed function call “mca_bucket_alloc”.


Hmm, I don't get the exact point from the above long sentence. Could you please describe a bit more specific where the problem is at exact line number of the code?

* Thus use a more appropriate label instead.

So far I am not convinced the modified label is more appropriate.


* Delete a redundant check.

Where is the location of the redundant check?




This issue was detected by using the Coccinelle software.

Just curious, what is the warning reported by the mentioned software ?



Fixes: cafe563591446cf80bfbc2fe3bc72a2e36cf1060 ("bcache: A block layer cache")
Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
---
  drivers/md/bcache/btree.c | 11 +++++------
  1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index 147c493a989a..166afd7ec499 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -921,18 +921,18 @@ static struct btree *mca_alloc(struct cache_set *c, struct btree_op *op,
  		if (!mca_reap(b, 0, false)) {
  			mca_data_alloc(b, k, __GFP_NOWARN|GFP_NOIO);
  			if (!b->keys.set[0].data)
-				goto err;
+				goto unlock;
  			else
  				goto out;
  		}

  	b = mca_bucket_alloc(c, k, __GFP_NOWARN|GFP_NOIO);
  	if (!b)
-		goto err;
+		goto unlock;

  	BUG_ON(!down_write_trylock(&b->lock));
  	if (!b->keys.set->data)
-		goto err;
+		goto unlock;
  out:
  	BUG_ON(b->io_mutex.count != 1);

@@ -955,9 +955,8 @@ static struct btree *mca_alloc(struct cache_set *c, struct btree_op *op,
  				    &b->c->expensive_debug_checks);

  	return b;
-err:
-	if (b)
-		rw_unlock(true, b);
+unlock:
+	rw_unlock(true, b);

If b is NULL, is it a bit fishing to send the NULL pointer into rw_unlock() ?


Thanks in advance for more information.


Coly Li




[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux