* [PATCH] p54: Fix an error handling path in p54spi_probe()
@ 2022-06-12 19:54 Christophe JAILLET
2022-06-12 20:45 ` Larry Finger
2022-06-12 20:45 ` Christian Lamparter
0 siblings, 2 replies; 4+ messages in thread
From: Christophe JAILLET @ 2022-06-12 19:54 UTC (permalink / raw)
To: Christian Lamparter, Kalle Valo, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, John W. Linville
Cc: linux-kernel, kernel-janitors, Christophe JAILLET,
Christian Lamparter, linux-wireless, netdev
If an error occurs after a successful call to p54spi_request_firmware(), it
must be undone by a corresponding release_firmware() as already done in
the error handling path of p54spi_request_firmware() and in the .remove()
function.
Add the missing call in the error handling path and update some goto
label accordingly.
Fixes: cd8d3d321285 ("p54spi: p54spi driver")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/net/wireless/intersil/p54/p54spi.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/intersil/p54/p54spi.c b/drivers/net/wireless/intersil/p54/p54spi.c
index f99b7ba69fc3..679ac164c994 100644
--- a/drivers/net/wireless/intersil/p54/p54spi.c
+++ b/drivers/net/wireless/intersil/p54/p54spi.c
@@ -650,14 +650,16 @@ static int p54spi_probe(struct spi_device *spi)
ret = p54spi_request_eeprom(hw);
if (ret)
- goto err_free_common;
+ goto err_release_firmaware;
ret = p54_register_common(hw, &priv->spi->dev);
if (ret)
- goto err_free_common;
+ goto err_release_firmaware;
return 0;
+err_release_firmaware:
+ release_firmware(priv->firmware);
err_free_common:
free_irq(gpio_to_irq(p54spi_gpio_irq), spi);
err_free_gpio_irq:
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] p54: Fix an error handling path in p54spi_probe()
2022-06-12 19:54 [PATCH] p54: Fix an error handling path in p54spi_probe() Christophe JAILLET
@ 2022-06-12 20:45 ` Larry Finger
2022-06-12 20:45 ` Christian Lamparter
1 sibling, 0 replies; 4+ messages in thread
From: Larry Finger @ 2022-06-12 20:45 UTC (permalink / raw)
To: Christophe JAILLET, Christian Lamparter, Kalle Valo,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
John W. Linville
Cc: linux-kernel, kernel-janitors, Christian Lamparter,
linux-wireless, netdev
On 6/12/22 14:54, Christophe JAILLET wrote:
> If an error occurs after a successful call to p54spi_request_firmware(), it
> must be undone by a corresponding release_firmware() as already done in
> the error handling path of p54spi_request_firmware() and in the .remove()
> function.
>
> Add the missing call in the error handling path and update some goto
> label accordingly.
>
> Fixes: cd8d3d321285 ("p54spi: p54spi driver")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> drivers/net/wireless/intersil/p54/p54spi.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/intersil/p54/p54spi.c b/drivers/net/wireless/intersil/p54/p54spi.c
> index f99b7ba69fc3..679ac164c994 100644
> --- a/drivers/net/wireless/intersil/p54/p54spi.c
> +++ b/drivers/net/wireless/intersil/p54/p54spi.c
> @@ -650,14 +650,16 @@ static int p54spi_probe(struct spi_device *spi)
>
> ret = p54spi_request_eeprom(hw);
> if (ret)
> - goto err_free_common;
> + goto err_release_firmaware;
>
> ret = p54_register_common(hw, &priv->spi->dev);
> if (ret)
> - goto err_free_common;
> + goto err_release_firmaware;
>
> return 0;
>
> +err_release_firmaware:
> + release_firmware(priv->firmware);
> err_free_common:
> free_irq(gpio_to_irq(p54spi_gpio_irq), spi);
> err_free_gpio_irq:
Why "err_release_firmaware" rather than "err_release_firmware"? Otherwise the
patch looks good.
Larry
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] p54: Fix an error handling path in p54spi_probe()
2022-06-12 19:54 [PATCH] p54: Fix an error handling path in p54spi_probe() Christophe JAILLET
2022-06-12 20:45 ` Larry Finger
@ 2022-06-12 20:45 ` Christian Lamparter
2022-06-12 21:02 ` Christophe JAILLET
1 sibling, 1 reply; 4+ messages in thread
From: Christian Lamparter @ 2022-06-12 20:45 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Kalle Valo, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, John W. Linville, linux-kernel, kernel-janitors,
Christian Lamparter, linux-wireless, Netdev
Hi,
On Sun, Jun 12, 2022 at 9:55 PM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> If an error occurs after a successful call to p54spi_request_firmware(), it
> must be undone by a corresponding release_firmware()
Yes, good catch. That makes sense.
> as already done in the error handling path of p54spi_request_firmware() and in
> the .remove() function.
>
> Add the missing call in the error handling path and update some goto
> label accordingly.
From what I know, "release_firmware(some *fw)" includes a check for
*fw != NULL already.
we could just add a single release_firmware(priv->firmware) to any of the error
paths labels (i.e.: err_free_common) and then we remove the extra
release_firmware(...) in p54spi_request_firmware so that we don't try to free
it twice.
(This also skips the need for having "err_release_firmaware" .. which
unfortunately has a small typo)
Regards,
Christian
> Fixes: cd8d3d321285 ("p54spi: p54spi driver")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> drivers/net/wireless/intersil/p54/p54spi.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/intersil/p54/p54spi.c b/drivers/net/wireless/intersil/p54/p54spi.c
> index f99b7ba69fc3..679ac164c994 100644
> --- a/drivers/net/wireless/intersil/p54/p54spi.c
> +++ b/drivers/net/wireless/intersil/p54/p54spi.c
> @@ -650,14 +650,16 @@ static int p54spi_probe(struct spi_device *spi)
>
> ret = p54spi_request_eeprom(hw);
> if (ret)
> - goto err_free_common;
> + goto err_release_firmaware;
>
> ret = p54_register_common(hw, &priv->spi->dev);
> if (ret)
> - goto err_free_common;
> + goto err_release_firmaware;
>
> return 0;
>
> +err_release_firmaware:
> + release_firmware(priv->firmware);
> err_free_common:
> free_irq(gpio_to_irq(p54spi_gpio_irq), spi);
> err_free_gpio_irq:
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] p54: Fix an error handling path in p54spi_probe()
2022-06-12 20:45 ` Christian Lamparter
@ 2022-06-12 21:02 ` Christophe JAILLET
0 siblings, 0 replies; 4+ messages in thread
From: Christophe JAILLET @ 2022-06-12 21:02 UTC (permalink / raw)
To: Christian Lamparter
Cc: Kalle Valo, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, John W. Linville, linux-kernel, kernel-janitors,
Christian Lamparter, linux-wireless, Netdev
Le 12/06/2022 à 22:45, Christian Lamparter a écrit :
> Hi,
>
> On Sun, Jun 12, 2022 at 9:55 PM Christophe JAILLET
> <christophe.jaillet@wanadoo.fr> wrote:
>>
>> If an error occurs after a successful call to p54spi_request_firmware(), it
>> must be undone by a corresponding release_firmware()
>
> Yes, good catch. That makes sense.
>
>> as already done in the error handling path of p54spi_request_firmware() and in
>> the .remove() function.
>>
>> Add the missing call in the error handling path and update some goto
>> label accordingly.
>
> From what I know, "release_firmware(some *fw)" includes a check for
> *fw != NULL already.
Correct.
>
> we could just add a single release_firmware(priv->firmware) to any of the error
Not my favorite style, but if it is preferred this way, NP.
> paths labels (i.e.: err_free_common) and then we remove the extra
> release_firmware(...) in p54spi_request_firmware so that we don't try to free
> it twice.
Sure. I'll add a comment to explain where is is released in case of error.
>
> (This also skips the need for having "err_release_firmaware" .. which
> unfortunately has a small typo)
>
> Regards,
> Christian
>
>> Fixes: cd8d3d321285 ("p54spi: p54spi driver")
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> ---
>> drivers/net/wireless/intersil/p54/p54spi.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/intersil/p54/p54spi.c b/drivers/net/wireless/intersil/p54/p54spi.c
>> index f99b7ba69fc3..679ac164c994 100644
>> --- a/drivers/net/wireless/intersil/p54/p54spi.c
>> +++ b/drivers/net/wireless/intersil/p54/p54spi.c
>> @@ -650,14 +650,16 @@ static int p54spi_probe(struct spi_device *spi)
>>
>> ret = p54spi_request_eeprom(hw);
>> if (ret)
>> - goto err_free_common;
>> + goto err_release_firmaware;
>>
>> ret = p54_register_common(hw, &priv->spi->dev);
>> if (ret)
>> - goto err_free_common;
>> + goto err_release_firmaware;
>>
>> return 0;
>>
>> +err_release_firmaware:
>> + release_firmware(priv->firmware);
>> err_free_common:
>> free_irq(gpio_to_irq(p54spi_gpio_irq), spi);
>> err_free_gpio_irq:
>> --
>> 2.34.1
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-12 21:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-12 19:54 [PATCH] p54: Fix an error handling path in p54spi_probe() Christophe JAILLET
2022-06-12 20:45 ` Larry Finger
2022-06-12 20:45 ` Christian Lamparter
2022-06-12 21:02 ` Christophe JAILLET
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).