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
