linux-kernel - Re: [PATCH 2/4] pwm: omap-dmtimer: simplify error handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 11 Nov 2019 14:32:30 +0100
From:   Markus Elfring <Markus.Elfring@....de>
To:     Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
        linux-pwm@...r.kernel.org
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Neil Brown <neilb@...e.de>,
        Neil Armstrong <narmstrong@...libre.com>,
        kernel@...gutronix.de, kernel-janitors@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/4] pwm: omap-dmtimer: simplify error handling

> Implementation note: The put: label was never reached without a goto and
> ret being unequal to 0, so the removed return statement is fine.

This can look fine (in principle) because the label was repositioned here.
Do you really want to call the function “of_node_put” at two places now?


> +++ b/drivers/pwm/pwm-omap-dmtimer.c
…
>  	omap = devm_kzalloc(&pdev->dev, sizeof(*omap), GFP_KERNEL);
>  	if (!omap) {
> -		pdata->free(dm_timer);
> -		return -ENOMEM;
> +		ret = -ENOMEM;
> +		goto err_alloc_omap;
>  	}
…

I suggest to reconsider your label name selection according to
the Linux coding style.


> @@ -339,13 +334,28 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
…
> +err_pwmchip_add:
> +
> +	/*
> +	 * *omap is allocated using devm_kzalloc,
> +	 * so no free necessary here
> +	 */
> +err_alloc_omap:
> +
> +	pdata->free(dm_timer);

Would the use of the label “free_dm_timer” be more appropriate?


> +put:
> +	of_node_put(timer);
…

Can the label “put_node” be nicer?

Regards,
Markus