On Tue, 9 Jun 2026, Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> Date: Tue, 9 Jun 2026 21:54:38 +0200
>
> Use additional labels so that a bit of exception handling can be better
> reused at the end of three if branches.
>
> This issue was detected by using the Coccinelle software.
Hi
I think that jumping into a nested block isn't good practice and it makes
the code harder to read and maintain. I would redo the patches so that
they jump to the end of the topmost function block, for example:
static struct hash_cell *alloc_cell(const char *name, const char *uuid,
struct mapped_device *md)
{
struct hash_cell *hc;
hc = kmalloc_obj(*hc);
if (!hc)
goto ret;
...
hc->name = kstrdup(name, GFP_KERNEL);
if (!hc->name)
goto free_hc_ret;
...
hc->uuid = kstrdup(uuid, GFP_KERNEL);
if (!hc->uuid)
goto free_hc_name_ret;
...
return hc;
free_hc_name_ret:
kfree(hc->name);
free_hc_ret:
kfree(hc);
ret:
return NULL;
}
Mikulas
> Signed-off-by: Markus Elfring <[email protected]>
> ---
> drivers/md/dm-ioctl.c | 7 +++----
> drivers/md/dm-raid1.c | 4 ++--
> drivers/md/dm-stripe.c | 7 +++----
> 3 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index ac77dc0ca225..9537e60f94c3 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -227,10 +227,8 @@ static struct hash_cell *alloc_cell(const char *name,
> const char *uuid,
> return NULL;
>
> hc->name = kstrdup(name, GFP_KERNEL);
> - if (!hc->name) {
> - kfree(hc);
> - return NULL;
> - }
> + if (!hc->name)
> + goto free_hc;
>
> if (!uuid)
> hc->uuid = NULL;
> @@ -239,6 +237,7 @@ static struct hash_cell *alloc_cell(const char *name,
> const char *uuid,
> hc->uuid = kstrdup(uuid, GFP_KERNEL);
> if (!hc->uuid) {
> kfree(hc->name);
> +free_hc:
> kfree(hc);
> return NULL;
> }
> diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c
> index de5c00704e69..359ca80a67b3 100644
> --- a/drivers/md/dm-raid1.c
> +++ b/drivers/md/dm-raid1.c
> @@ -915,8 +915,7 @@ static struct mirror_set *alloc_context(unsigned int
> nr_mirrors,
> ms->io_client = dm_io_client_create();
> if (IS_ERR(ms->io_client)) {
> ti->error = "Error creating dm_io client";
> - kfree(ms);
> - return NULL;
> + goto free_ms;
> }
>
> ms->rh = dm_region_hash_create(ms, dispatch_bios, wakeup_mirrord,
> @@ -926,6 +925,7 @@ static struct mirror_set *alloc_context(unsigned int
> nr_mirrors,
> if (IS_ERR(ms->rh)) {
> ti->error = "Error creating dirty region hash";
> dm_io_client_destroy(ms->io_client);
> +free_ms:
> kfree(ms);
> return NULL;
> }
> diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
> index 750865fd3ae7..58fa0badc90f 100644
> --- a/drivers/md/dm-stripe.c
> +++ b/drivers/md/dm-stripe.c
> @@ -148,10 +148,8 @@ static int stripe_ctr(struct dm_target *ti, unsigned int
> argc, char **argv)
> sc->stripes_shift = __ffs(stripes);
>
> r = dm_set_target_max_io_len(ti, chunk_size);
> - if (r) {
> - kfree(sc);
> - return r;
> - }
> + if (r)
> + goto free_sc;
>
> ti->num_flush_bios = stripes;
> ti->num_discard_bios = stripes;
> @@ -176,6 +174,7 @@ static int stripe_ctr(struct dm_target *ti, unsigned int
> argc, char **argv)
> ti->error = "Couldn't parse stripe destination";
> while (i--)
> dm_put_device(ti, sc->stripe[i].dev);
> +free_sc:
> kfree(sc);
> return r;
> }
> --
> 2.54.0
>
