* [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
@ 2017-08-22 17:43 ` SF Markus Elfring
0 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-08-22 17:43 UTC (permalink / raw)
To: alsa-devel, Jaroslav Kysela, Julia Lawall, Takashi Iwai
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 22 Aug 2017 19:37:26 +0200
Add jump targets so that a bit of exception handling can be better reused
at the end of this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
sound/pci/via82xx_modem.c | 33 +++++++++++++++++++--------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/sound/pci/via82xx_modem.c b/sound/pci/via82xx_modem.c
index 55f79b2599e7..8659776887b2 100644
--- a/sound/pci/via82xx_modem.c
+++ b/sound/pci/via82xx_modem.c
@@ -1110,8 +1110,8 @@ static int snd_via82xx_create(struct snd_card *card,
return err;
if ((chip = kzalloc(sizeof(*chip), GFP_KERNEL)) == NULL) {
- pci_disable_device(pci);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto disable_device;
}
spin_lock_init(&chip->reg_lock);
@@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
if ((err = pci_request_regions(pci, card->driver)) < 0) {
kfree(chip);
- pci_disable_device(pci);
- return err;
+ goto disable_device;
}
chip->port = pci_resource_start(pci, 0);
if (request_irq(pci->irq, snd_via82xx_interrupt, IRQF_SHARED,
KBUILD_MODNAME, chip)) {
dev_err(card->dev, "unable to grab IRQ %d\n", pci->irq);
- snd_via82xx_free(chip);
- return -EBUSY;
+ err = -EBUSY;
+ goto free_chip;
}
chip->irq = pci->irq;
if (ac97_clock >= 8000 && ac97_clock <= 48000)
chip->ac97_clock = ac97_clock;
synchronize_irq(chip->irq);
- if ((err = snd_via82xx_chip_init(chip)) < 0) {
- snd_via82xx_free(chip);
- return err;
- }
+ err = snd_via82xx_chip_init(chip);
+ if (err < 0)
+ goto free_chip;
- if ((err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops)) < 0) {
- snd_via82xx_free(chip);
- return err;
- }
+ err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops);
+ if (err < 0)
+ goto free_chip;
/* The 8233 ac97 controller does not implement the master bit
* in the pci command register. IMHO this is a violation of the PCI spec.
@@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
*r_via = chip;
return 0;
+
+disable_device:
+ pci_disable_device(pci);
+ goto exit;
+free_chip:
+ snd_via82xx_free(chip);
+exit:
+ return err;
}
--
2.14.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
@ 2017-08-22 17:43 ` SF Markus Elfring
0 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-08-22 17:43 UTC (permalink / raw)
To: alsa-devel, Jaroslav Kysela, Julia Lawall, Takashi Iwai
Cc: kernel-janitors, LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 22 Aug 2017 19:37:26 +0200
Add jump targets so that a bit of exception handling can be better reused
at the end of this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
sound/pci/via82xx_modem.c | 33 +++++++++++++++++++--------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/sound/pci/via82xx_modem.c b/sound/pci/via82xx_modem.c
index 55f79b2599e7..8659776887b2 100644
--- a/sound/pci/via82xx_modem.c
+++ b/sound/pci/via82xx_modem.c
@@ -1110,8 +1110,8 @@ static int snd_via82xx_create(struct snd_card *card,
return err;
if ((chip = kzalloc(sizeof(*chip), GFP_KERNEL)) = NULL) {
- pci_disable_device(pci);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto disable_device;
}
spin_lock_init(&chip->reg_lock);
@@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
if ((err = pci_request_regions(pci, card->driver)) < 0) {
kfree(chip);
- pci_disable_device(pci);
- return err;
+ goto disable_device;
}
chip->port = pci_resource_start(pci, 0);
if (request_irq(pci->irq, snd_via82xx_interrupt, IRQF_SHARED,
KBUILD_MODNAME, chip)) {
dev_err(card->dev, "unable to grab IRQ %d\n", pci->irq);
- snd_via82xx_free(chip);
- return -EBUSY;
+ err = -EBUSY;
+ goto free_chip;
}
chip->irq = pci->irq;
if (ac97_clock >= 8000 && ac97_clock <= 48000)
chip->ac97_clock = ac97_clock;
synchronize_irq(chip->irq);
- if ((err = snd_via82xx_chip_init(chip)) < 0) {
- snd_via82xx_free(chip);
- return err;
- }
+ err = snd_via82xx_chip_init(chip);
+ if (err < 0)
+ goto free_chip;
- if ((err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops)) < 0) {
- snd_via82xx_free(chip);
- return err;
- }
+ err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops);
+ if (err < 0)
+ goto free_chip;
/* The 8233 ac97 controller does not implement the master bit
* in the pci command register. IMHO this is a violation of the PCI spec.
@@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
*r_via = chip;
return 0;
+
+disable_device:
+ pci_disable_device(pci);
+ goto exit;
+free_chip:
+ snd_via82xx_free(chip);
+exit:
+ return err;
}
--
2.14.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
@ 2017-08-22 17:43 ` SF Markus Elfring
0 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-08-22 17:43 UTC (permalink / raw)
To: alsa-devel, Jaroslav Kysela, Julia Lawall, Takashi Iwai
Cc: kernel-janitors, LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 22 Aug 2017 19:37:26 +0200
Add jump targets so that a bit of exception handling can be better reused
at the end of this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
sound/pci/via82xx_modem.c | 33 +++++++++++++++++++--------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/sound/pci/via82xx_modem.c b/sound/pci/via82xx_modem.c
index 55f79b2599e7..8659776887b2 100644
--- a/sound/pci/via82xx_modem.c
+++ b/sound/pci/via82xx_modem.c
@@ -1110,8 +1110,8 @@ static int snd_via82xx_create(struct snd_card *card,
return err;
if ((chip = kzalloc(sizeof(*chip), GFP_KERNEL)) == NULL) {
- pci_disable_device(pci);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto disable_device;
}
spin_lock_init(&chip->reg_lock);
@@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
if ((err = pci_request_regions(pci, card->driver)) < 0) {
kfree(chip);
- pci_disable_device(pci);
- return err;
+ goto disable_device;
}
chip->port = pci_resource_start(pci, 0);
if (request_irq(pci->irq, snd_via82xx_interrupt, IRQF_SHARED,
KBUILD_MODNAME, chip)) {
dev_err(card->dev, "unable to grab IRQ %d\n", pci->irq);
- snd_via82xx_free(chip);
- return -EBUSY;
+ err = -EBUSY;
+ goto free_chip;
}
chip->irq = pci->irq;
if (ac97_clock >= 8000 && ac97_clock <= 48000)
chip->ac97_clock = ac97_clock;
synchronize_irq(chip->irq);
- if ((err = snd_via82xx_chip_init(chip)) < 0) {
- snd_via82xx_free(chip);
- return err;
- }
+ err = snd_via82xx_chip_init(chip);
+ if (err < 0)
+ goto free_chip;
- if ((err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops)) < 0) {
- snd_via82xx_free(chip);
- return err;
- }
+ err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops);
+ if (err < 0)
+ goto free_chip;
/* The 8233 ac97 controller does not implement the master bit
* in the pci command register. IMHO this is a violation of the PCI spec.
@@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
*r_via = chip;
return 0;
+
+disable_device:
+ pci_disable_device(pci);
+ goto exit;
+free_chip:
+ snd_via82xx_free(chip);
+exit:
+ return err;
}
--
2.14.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
2017-08-22 17:43 ` SF Markus Elfring
(?)
@ 2017-08-22 17:57 ` Takashi Iwai
-1 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2017-08-22 17:57 UTC (permalink / raw)
To: SF Markus Elfring
Cc: alsa-devel, Julia Lawall, Jaroslav Kysela, kernel-janitors, LKML
On Tue, 22 Aug 2017 19:43:08 +0200,
SF Markus Elfring wrote:
>
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 22 Aug 2017 19:37:26 +0200
>
> Add jump targets so that a bit of exception handling can be better reused
> at the end of this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> sound/pci/via82xx_modem.c | 33 +++++++++++++++++++--------------
> 1 file changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/sound/pci/via82xx_modem.c b/sound/pci/via82xx_modem.c
> index 55f79b2599e7..8659776887b2 100644
> --- a/sound/pci/via82xx_modem.c
> +++ b/sound/pci/via82xx_modem.c
> @@ -1110,8 +1110,8 @@ static int snd_via82xx_create(struct snd_card *card,
> return err;
>
> if ((chip = kzalloc(sizeof(*chip), GFP_KERNEL)) == NULL) {
> - pci_disable_device(pci);
> - return -ENOMEM;
> + err = -ENOMEM;
> + goto disable_device;
> }
>
> spin_lock_init(&chip->reg_lock);
> @@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
>
> if ((err = pci_request_regions(pci, card->driver)) < 0) {
> kfree(chip);
> - pci_disable_device(pci);
> - return err;
> + goto disable_device;
> }
> chip->port = pci_resource_start(pci, 0);
> if (request_irq(pci->irq, snd_via82xx_interrupt, IRQF_SHARED,
> KBUILD_MODNAME, chip)) {
> dev_err(card->dev, "unable to grab IRQ %d\n", pci->irq);
> - snd_via82xx_free(chip);
> - return -EBUSY;
> + err = -EBUSY;
> + goto free_chip;
> }
> chip->irq = pci->irq;
> if (ac97_clock >= 8000 && ac97_clock <= 48000)
> chip->ac97_clock = ac97_clock;
> synchronize_irq(chip->irq);
>
> - if ((err = snd_via82xx_chip_init(chip)) < 0) {
> - snd_via82xx_free(chip);
> - return err;
> - }
> + err = snd_via82xx_chip_init(chip);
> + if (err < 0)
> + goto free_chip;
>
> - if ((err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops)) < 0) {
> - snd_via82xx_free(chip);
> - return err;
> - }
> + err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops);
> + if (err < 0)
> + goto free_chip;
>
> /* The 8233 ac97 controller does not implement the master bit
> * in the pci command register. IMHO this is a violation of the PCI spec.
> @@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
>
> *r_via = chip;
> return 0;
> +
> +disable_device:
> + pci_disable_device(pci);
> + goto exit;
> +free_chip:
> + snd_via82xx_free(chip);
> +exit:
> + return err;
Doubly goto doesn't look like an improvement.
Takashi
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
@ 2017-08-22 17:57 ` Takashi Iwai
0 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2017-08-22 17:57 UTC (permalink / raw)
To: SF Markus Elfring
Cc: alsa-devel, Julia Lawall, Jaroslav Kysela, kernel-janitors, LKML
On Tue, 22 Aug 2017 19:43:08 +0200,
SF Markus Elfring wrote:
>
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 22 Aug 2017 19:37:26 +0200
>
> Add jump targets so that a bit of exception handling can be better reused
> at the end of this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> sound/pci/via82xx_modem.c | 33 +++++++++++++++++++--------------
> 1 file changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/sound/pci/via82xx_modem.c b/sound/pci/via82xx_modem.c
> index 55f79b2599e7..8659776887b2 100644
> --- a/sound/pci/via82xx_modem.c
> +++ b/sound/pci/via82xx_modem.c
> @@ -1110,8 +1110,8 @@ static int snd_via82xx_create(struct snd_card *card,
> return err;
>
> if ((chip = kzalloc(sizeof(*chip), GFP_KERNEL)) = NULL) {
> - pci_disable_device(pci);
> - return -ENOMEM;
> + err = -ENOMEM;
> + goto disable_device;
> }
>
> spin_lock_init(&chip->reg_lock);
> @@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
>
> if ((err = pci_request_regions(pci, card->driver)) < 0) {
> kfree(chip);
> - pci_disable_device(pci);
> - return err;
> + goto disable_device;
> }
> chip->port = pci_resource_start(pci, 0);
> if (request_irq(pci->irq, snd_via82xx_interrupt, IRQF_SHARED,
> KBUILD_MODNAME, chip)) {
> dev_err(card->dev, "unable to grab IRQ %d\n", pci->irq);
> - snd_via82xx_free(chip);
> - return -EBUSY;
> + err = -EBUSY;
> + goto free_chip;
> }
> chip->irq = pci->irq;
> if (ac97_clock >= 8000 && ac97_clock <= 48000)
> chip->ac97_clock = ac97_clock;
> synchronize_irq(chip->irq);
>
> - if ((err = snd_via82xx_chip_init(chip)) < 0) {
> - snd_via82xx_free(chip);
> - return err;
> - }
> + err = snd_via82xx_chip_init(chip);
> + if (err < 0)
> + goto free_chip;
>
> - if ((err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops)) < 0) {
> - snd_via82xx_free(chip);
> - return err;
> - }
> + err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops);
> + if (err < 0)
> + goto free_chip;
>
> /* The 8233 ac97 controller does not implement the master bit
> * in the pci command register. IMHO this is a violation of the PCI spec.
> @@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
>
> *r_via = chip;
> return 0;
> +
> +disable_device:
> + pci_disable_device(pci);
> + goto exit;
> +free_chip:
> + snd_via82xx_free(chip);
> +exit:
> + return err;
Doubly goto doesn't look like an improvement.
Takashi
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
@ 2017-08-22 17:57 ` Takashi Iwai
0 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2017-08-22 17:57 UTC (permalink / raw)
To: SF Markus Elfring
Cc: alsa-devel, Julia Lawall, Jaroslav Kysela, kernel-janitors, LKML
On Tue, 22 Aug 2017 19:43:08 +0200,
SF Markus Elfring wrote:
>
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 22 Aug 2017 19:37:26 +0200
>
> Add jump targets so that a bit of exception handling can be better reused
> at the end of this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> sound/pci/via82xx_modem.c | 33 +++++++++++++++++++--------------
> 1 file changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/sound/pci/via82xx_modem.c b/sound/pci/via82xx_modem.c
> index 55f79b2599e7..8659776887b2 100644
> --- a/sound/pci/via82xx_modem.c
> +++ b/sound/pci/via82xx_modem.c
> @@ -1110,8 +1110,8 @@ static int snd_via82xx_create(struct snd_card *card,
> return err;
>
> if ((chip = kzalloc(sizeof(*chip), GFP_KERNEL)) == NULL) {
> - pci_disable_device(pci);
> - return -ENOMEM;
> + err = -ENOMEM;
> + goto disable_device;
> }
>
> spin_lock_init(&chip->reg_lock);
> @@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
>
> if ((err = pci_request_regions(pci, card->driver)) < 0) {
> kfree(chip);
> - pci_disable_device(pci);
> - return err;
> + goto disable_device;
> }
> chip->port = pci_resource_start(pci, 0);
> if (request_irq(pci->irq, snd_via82xx_interrupt, IRQF_SHARED,
> KBUILD_MODNAME, chip)) {
> dev_err(card->dev, "unable to grab IRQ %d\n", pci->irq);
> - snd_via82xx_free(chip);
> - return -EBUSY;
> + err = -EBUSY;
> + goto free_chip;
> }
> chip->irq = pci->irq;
> if (ac97_clock >= 8000 && ac97_clock <= 48000)
> chip->ac97_clock = ac97_clock;
> synchronize_irq(chip->irq);
>
> - if ((err = snd_via82xx_chip_init(chip)) < 0) {
> - snd_via82xx_free(chip);
> - return err;
> - }
> + err = snd_via82xx_chip_init(chip);
> + if (err < 0)
> + goto free_chip;
>
> - if ((err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops)) < 0) {
> - snd_via82xx_free(chip);
> - return err;
> - }
> + err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops);
> + if (err < 0)
> + goto free_chip;
>
> /* The 8233 ac97 controller does not implement the master bit
> * in the pci command register. IMHO this is a violation of the PCI spec.
> @@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
>
> *r_via = chip;
> return 0;
> +
> +disable_device:
> + pci_disable_device(pci);
> + goto exit;
> +free_chip:
> + snd_via82xx_free(chip);
> +exit:
> + return err;
Doubly goto doesn't look like an improvement.
Takashi
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
2017-08-22 17:43 ` SF Markus Elfring
(?)
@ 2017-08-22 19:07 ` Dan Carpenter
-1 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2017-08-22 19:07 UTC (permalink / raw)
To: SF Markus Elfring
Cc: alsa-devel, Jaroslav Kysela, Julia Lawall, Takashi Iwai, LKML,
kernel-janitors
On Tue, Aug 22, 2017 at 07:43:08PM +0200, SF Markus Elfring wrote:
> @@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
>
> if ((err = pci_request_regions(pci, card->driver)) < 0) {
> kfree(chip);
> - pci_disable_device(pci);
> - return err;
> + goto disable_device;
> }
[ snip ]
> @@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
>
> *r_via = chip;
> return 0;
> +
> +disable_device:
> + pci_disable_device(pci);
> + goto exit;
Why is this "goto exit" here? It's leaking now.
> +free_chip:
> + snd_via82xx_free(chip);
> +exit:
> + return err;
> }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
@ 2017-08-22 19:07 ` Dan Carpenter
0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2017-08-22 19:07 UTC (permalink / raw)
To: SF Markus Elfring
Cc: alsa-devel, kernel-janitors, Takashi Iwai, LKML, Julia Lawall
On Tue, Aug 22, 2017 at 07:43:08PM +0200, SF Markus Elfring wrote:
> @@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
>
> if ((err = pci_request_regions(pci, card->driver)) < 0) {
> kfree(chip);
> - pci_disable_device(pci);
> - return err;
> + goto disable_device;
> }
[ snip ]
> @@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
>
> *r_via = chip;
> return 0;
> +
> +disable_device:
> + pci_disable_device(pci);
> + goto exit;
Why is this "goto exit" here? It's leaking now.
> +free_chip:
> + snd_via82xx_free(chip);
> +exit:
> + return err;
> }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
@ 2017-08-22 19:07 ` Dan Carpenter
0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2017-08-22 19:07 UTC (permalink / raw)
To: SF Markus Elfring
Cc: alsa-devel, kernel-janitors, Takashi Iwai, LKML, Julia Lawall
On Tue, Aug 22, 2017 at 07:43:08PM +0200, SF Markus Elfring wrote:
> @@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
>
> if ((err = pci_request_regions(pci, card->driver)) < 0) {
> kfree(chip);
> - pci_disable_device(pci);
> - return err;
> + goto disable_device;
> }
[ snip ]
> @@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
>
> *r_via = chip;
> return 0;
> +
> +disable_device:
> + pci_disable_device(pci);
> + goto exit;
Why is this "goto exit" here? It's leaking now.
> +free_chip:
> + snd_via82xx_free(chip);
> +exit:
> + return err;
> }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
2017-08-22 19:07 ` Dan Carpenter
(?)
@ 2017-08-22 20:05 ` SF Markus Elfring
-1 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-08-22 20:05 UTC (permalink / raw)
To: Dan Carpenter, alsa-devel
Cc: Jaroslav Kysela, Julia Lawall, Takashi Iwai, LKML, kernel-janitors
>> @@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
>>
>> if ((err = pci_request_regions(pci, card->driver)) < 0) {
>> kfree(chip);
>> - pci_disable_device(pci);
>> - return err;
>> + goto disable_device;
>> }
>
> [ snip ]
>
>> @@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
>>
>> *r_via = chip;
>> return 0;
>> +
>> +disable_device:
>> + pci_disable_device(pci);
>> + goto exit;
>
> Why is this "goto exit" here?
Should the same return statement be reached after the proposed refactoring?
Would you like to move such a function call to this source code place?
> It's leaking now.
How do you come to this conclusion?
>> +free_chip:
>> + snd_via82xx_free(chip);
>> +exit:
>> + return err;
>> }
Is this update suggestion worth for another look?
Regards,
Markus
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
@ 2017-08-22 20:05 ` SF Markus Elfring
0 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-08-22 20:05 UTC (permalink / raw)
To: Dan Carpenter, alsa-devel
Cc: Julia Lawall, kernel-janitors, Takashi Iwai, LKML
>> @@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
>>
>> if ((err = pci_request_regions(pci, card->driver)) < 0) {
>> kfree(chip);
>> - pci_disable_device(pci);
>> - return err;
>> + goto disable_device;
>> }
>
> [ snip ]
>
>> @@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
>>
>> *r_via = chip;
>> return 0;
>> +
>> +disable_device:
>> + pci_disable_device(pci);
>> + goto exit;
>
> Why is this "goto exit" here?
Should the same return statement be reached after the proposed refactoring?
Would you like to move such a function call to this source code place?
> It's leaking now.
How do you come to this conclusion?
>> +free_chip:
>> + snd_via82xx_free(chip);
>> +exit:
>> + return err;
>> }
Is this update suggestion worth for another look?
Regards,
Markus
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
@ 2017-08-22 20:05 ` SF Markus Elfring
0 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-08-22 20:05 UTC (permalink / raw)
To: Dan Carpenter, alsa-devel
Cc: Julia Lawall, kernel-janitors, Takashi Iwai, LKML
>> @@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
>>
>> if ((err = pci_request_regions(pci, card->driver)) < 0) {
>> kfree(chip);
>> - pci_disable_device(pci);
>> - return err;
>> + goto disable_device;
>> }
>
> [ snip ]
>
>> @@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
>>
>> *r_via = chip;
>> return 0;
>> +
>> +disable_device:
>> + pci_disable_device(pci);
>> + goto exit;
>
> Why is this "goto exit" here?
Should the same return statement be reached after the proposed refactoring?
Would you like to move such a function call to this source code place?
> It's leaking now.
How do you come to this conclusion?
>> +free_chip:
>> + snd_via82xx_free(chip);
>> +exit:
>> + return err;
>> }
Is this update suggestion worth for another look?
Regards,
Markus
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
2017-08-22 20:05 ` SF Markus Elfring
(?)
@ 2017-08-22 20:16 ` Dan Carpenter
-1 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2017-08-22 20:16 UTC (permalink / raw)
To: SF Markus Elfring
Cc: alsa-devel, Jaroslav Kysela, Julia Lawall, Takashi Iwai, LKML,
kernel-janitors
On Tue, Aug 22, 2017 at 10:05:37PM +0200, SF Markus Elfring wrote:
> >> @@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
> >>
> >> if ((err = pci_request_regions(pci, card->driver)) < 0) {
> >> kfree(chip);
> >> - pci_disable_device(pci);
> >> - return err;
> >> + goto disable_device;
Ah, you're right. It's not leaking. I thought you deleted the
kfree(chip);...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
@ 2017-08-22 20:16 ` Dan Carpenter
0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2017-08-22 20:16 UTC (permalink / raw)
To: SF Markus Elfring
Cc: alsa-devel, kernel-janitors, Takashi Iwai, LKML, Julia Lawall
On Tue, Aug 22, 2017 at 10:05:37PM +0200, SF Markus Elfring wrote:
> >> @@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
> >>
> >> if ((err = pci_request_regions(pci, card->driver)) < 0) {
> >> kfree(chip);
> >> - pci_disable_device(pci);
> >> - return err;
> >> + goto disable_device;
Ah, you're right. It's not leaking. I thought you deleted the
kfree(chip);...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
@ 2017-08-22 20:16 ` Dan Carpenter
0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2017-08-22 20:16 UTC (permalink / raw)
To: SF Markus Elfring
Cc: alsa-devel, kernel-janitors, Takashi Iwai, LKML, Julia Lawall
On Tue, Aug 22, 2017 at 10:05:37PM +0200, SF Markus Elfring wrote:
> >> @@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
> >>
> >> if ((err = pci_request_regions(pci, card->driver)) < 0) {
> >> kfree(chip);
> >> - pci_disable_device(pci);
> >> - return err;
> >> + goto disable_device;
Ah, you're right. It's not leaking. I thought you deleted the
kfree(chip);...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
2017-08-22 20:16 ` Dan Carpenter
@ 2017-08-22 20:19 ` Takashi Iwai
-1 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2017-08-22 20:19 UTC (permalink / raw)
To: Dan Carpenter
Cc: SF Markus Elfring, alsa-devel, kernel-janitors, LKML, Julia Lawall
On Tue, 22 Aug 2017 22:16:12 +0200,
Dan Carpenter wrote:
>
> On Tue, Aug 22, 2017 at 10:05:37PM +0200, SF Markus Elfring wrote:
> > >> @@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
> > >>
> > >> if ((err = pci_request_regions(pci, card->driver)) < 0) {
> > >> kfree(chip);
> > >> - pci_disable_device(pci);
> > >> - return err;
> > >> + goto disable_device;
>
> Ah, you're right. It's not leaking. I thought you deleted the
> kfree(chip);...
Well, this also suggests something wrong by the patch...
If you can't achieve it cleanly, better not to touch it in a
half-baked way.
Takashi
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [alsa-devel] [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
@ 2017-08-22 20:19 ` Takashi Iwai
0 siblings, 0 replies; 20+ messages in thread
From: Takashi Iwai @ 2017-08-22 20:19 UTC (permalink / raw)
To: Dan Carpenter
Cc: SF Markus Elfring, alsa-devel, kernel-janitors, LKML, Julia Lawall
On Tue, 22 Aug 2017 22:16:12 +0200,
Dan Carpenter wrote:
>
> On Tue, Aug 22, 2017 at 10:05:37PM +0200, SF Markus Elfring wrote:
> > >> @@ -1121,30 +1121,27 @@ static int snd_via82xx_create(struct snd_card *card,
> > >>
> > >> if ((err = pci_request_regions(pci, card->driver)) < 0) {
> > >> kfree(chip);
> > >> - pci_disable_device(pci);
> > >> - return err;
> > >> + goto disable_device;
>
> Ah, you're right. It's not leaking. I thought you deleted the
> kfree(chip);...
Well, this also suggests something wrong by the patch...
If you can't achieve it cleanly, better not to touch it in a
half-baked way.
Takashi
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
2017-08-22 17:57 ` Takashi Iwai
(?)
@ 2017-11-19 6:37 ` SF Markus Elfring
-1 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-11-19 6:37 UTC (permalink / raw)
To: Takashi Iwai, alsa-devel
Cc: Julia Lawall, Jaroslav Kysela, kernel-janitors, LKML
>> @@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
>>
>> *r_via = chip;
>> return 0;
>> +
>> +disable_device:
>> + pci_disable_device(pci);
>> + goto exit;
>> +free_chip:
>> + snd_via82xx_free(chip);
>> +exit:
>> + return err;
>
> Doubly goto doesn't look like an improvement.
Would you like to integrate another software update with the statement
“return err;” instead of “goto exit;” at this place?
Regards,
Markus
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
@ 2017-11-19 6:37 ` SF Markus Elfring
0 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-11-19 6:37 UTC (permalink / raw)
To: Takashi Iwai, alsa-devel; +Cc: Julia Lawall, kernel-janitors, LKML
>> @@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
>>
>> *r_via = chip;
>> return 0;
>> +
>> +disable_device:
>> + pci_disable_device(pci);
>> + goto exit;
>> +free_chip:
>> + snd_via82xx_free(chip);
>> +exit:
>> + return err;
>
> Doubly goto doesn't look like an improvement.
Would you like to integrate another software update with the statement
“return err;” instead of “goto exit;” at this place?
Regards,
Markus
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create()
@ 2017-11-19 6:37 ` SF Markus Elfring
0 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-11-19 6:37 UTC (permalink / raw)
To: Takashi Iwai, alsa-devel; +Cc: Julia Lawall, kernel-janitors, LKML
>> @@ -1153,6 +1150,14 @@ static int snd_via82xx_create(struct snd_card *card,
>>
>> *r_via = chip;
>> return 0;
>> +
>> +disable_device:
>> + pci_disable_device(pci);
>> + goto exit;
>> +free_chip:
>> + snd_via82xx_free(chip);
>> +exit:
>> + return err;
>
> Doubly goto doesn't look like an improvement.
Would you like to integrate another software update with the statement
“return err;” instead of “goto exit;” at this place?
Regards,
Markus
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-11-19 6:38 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-22 17:43 [PATCH] ALSA: via82xx: Use common error handling code in snd_via82xx_create() SF Markus Elfring
2017-08-22 17:43 ` SF Markus Elfring
2017-08-22 17:43 ` SF Markus Elfring
2017-08-22 17:57 ` Takashi Iwai
2017-08-22 17:57 ` Takashi Iwai
2017-08-22 17:57 ` Takashi Iwai
2017-11-19 6:37 ` SF Markus Elfring
2017-11-19 6:37 ` SF Markus Elfring
2017-11-19 6:37 ` SF Markus Elfring
2017-08-22 19:07 ` Dan Carpenter
2017-08-22 19:07 ` Dan Carpenter
2017-08-22 19:07 ` Dan Carpenter
2017-08-22 20:05 ` SF Markus Elfring
2017-08-22 20:05 ` SF Markus Elfring
2017-08-22 20:05 ` SF Markus Elfring
2017-08-22 20:16 ` Dan Carpenter
2017-08-22 20:16 ` Dan Carpenter
2017-08-22 20:16 ` Dan Carpenter
2017-08-22 20:19 ` [alsa-devel] " Takashi Iwai
2017-08-22 20:19 ` Takashi Iwai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.