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 <[email protected]>
> ---
> 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