[alsa-devel] [PATCH 0/3] ALSA-S3 SonicVibes: Fine-tuning for five function implementations
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 18 Nov 2017 16:25:43 +0100
A few update suggestions were taken into account from static source code analysis.
Markus Elfring (3): Adjust 15 function calls together with a variable assignment Use common error handling code in snd_sonic_probe() Use common error handling code in snd_sonicvibes_create()
sound/pci/sonicvibes.c | 144 +++++++++++++++++++++++++++---------------------- 1 file changed, 79 insertions(+), 65 deletions(-)
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 18 Nov 2017 16:08:23 +0100
The script "checkpatch.pl" pointed information out like the following.
ERROR: do not use assignment in if condition
Thus fix the affected source code places.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/pci/sonicvibes.c | 70 +++++++++++++++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 27 deletions(-)
diff --git a/sound/pci/sonicvibes.c b/sound/pci/sonicvibes.c index a8abb15e3c3a..7c238d908031 100644 --- a/sound/pci/sonicvibes.c +++ b/sound/pci/sonicvibes.c @@ -882,9 +882,9 @@ static const struct snd_pcm_ops snd_sonicvibes_capture_ops = { static int snd_sonicvibes_pcm(struct sonicvibes *sonic, int device) { struct snd_pcm *pcm; - int err; + int err = snd_pcm_new(sonic->card, "s3_86c617", device, 1, 1, &pcm);
- if ((err = snd_pcm_new(sonic->card, "s3_86c617", device, 1, 1, &pcm)) < 0) + if (err < 0) return err; if (snd_BUG_ON(!pcm)) return -EINVAL; @@ -1125,7 +1125,9 @@ static int snd_sonicvibes_mixer(struct sonicvibes *sonic) strcpy(card->mixername, "S3 SonicVibes");
for (idx = 0; idx < ARRAY_SIZE(snd_sonicvibes_controls); idx++) { - if ((err = snd_ctl_add(card, kctl = snd_ctl_new1(&snd_sonicvibes_controls[idx], sonic))) < 0) + kctl = snd_ctl_new1(&snd_sonicvibes_controls[idx], sonic); + err = snd_ctl_add(card, kctl); + if (err < 0) return err; switch (idx) { case 0: @@ -1256,7 +1258,8 @@ static int snd_sonicvibes_create(struct snd_card *card,
*rsonic = NULL; /* enable PCI device */ - if ((err = pci_enable_device(pci)) < 0) + err = pci_enable_device(pci); + if (err < 0) return err; /* check, if we can restrict PCI DMA transfers to 24 bits */ if (dma_set_mask(&pci->dev, DMA_BIT_MASK(24)) < 0 || @@ -1277,7 +1280,8 @@ static int snd_sonicvibes_create(struct snd_card *card, sonic->pci = pci; sonic->irq = -1;
- if ((err = pci_request_regions(pci, "S3 SonicVibes")) < 0) { + err = pci_request_regions(pci, "S3 SonicVibes"); + if (err < 0) { kfree(sonic); pci_disable_device(pci); return err; @@ -1319,14 +1323,17 @@ static int snd_sonicvibes_create(struct snd_card *card, pci_write_config_dword(pci, 0x40, dmaa); pci_write_config_dword(pci, 0x48, dmac);
- if ((sonic->res_dmaa = request_region(dmaa, 0x10, "S3 SonicVibes DDMA-A")) == NULL) { + sonic->res_dmaa = request_region(dmaa, 0x10, "S3 SonicVibes DDMA-A"); + if (!sonic->res_dmaa) { snd_sonicvibes_free(sonic); dev_err(card->dev, "unable to grab DDMA-A port at 0x%x-0x%x\n", dmaa, dmaa + 0x10 - 1); return -EBUSY; } - if ((sonic->res_dmac = request_region(dmac, 0x10, "S3 SonicVibes DDMA-C")) == NULL) { + + sonic->res_dmac = request_region(dmac, 0x10, "S3 SonicVibes DDMA-C"); + if (!sonic->res_dmac) { snd_sonicvibes_free(sonic); dev_err(card->dev, "unable to grab DDMA-C port at 0x%x-0x%x\n", @@ -1387,8 +1394,8 @@ static int snd_sonicvibes_create(struct snd_card *card, snd_sonicvibes_debug(sonic); #endif sonic->revision = snd_sonicvibes_in(sonic, SV_IREG_REVISION); - - if ((err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, sonic, &ops)) < 0) { + err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, sonic, &ops); + if (err < 0) { snd_sonicvibes_free(sonic); return err; } @@ -1437,9 +1444,13 @@ static int snd_sonicvibes_midi(struct sonicvibes *sonic, mpu->open_input = snd_sonicvibes_midi_input_open; mpu->close_input = snd_sonicvibes_midi_input_close; dir = &rmidi->streams[SNDRV_RAWMIDI_STREAM_OUTPUT]; - for (idx = 0; idx < ARRAY_SIZE(snd_sonicvibes_midi_controls); idx++) - if ((err = snd_ctl_add(card, snd_ctl_new1(&snd_sonicvibes_midi_controls[idx], sonic))) < 0) + for (idx = 0; idx < ARRAY_SIZE(snd_sonicvibes_midi_controls); idx++) { + err = snd_ctl_add(card, + snd_ctl_new1(&snd_sonicvibes_midi_controls[idx], + sonic)); + if (err < 0) return err; + } return 0; }
@@ -1471,10 +1482,10 @@ static int snd_sonic_probe(struct pci_dev *pci, return -ENODEV; } } - if ((err = snd_sonicvibes_create(card, pci, - reverb[dev] ? 1 : 0, - mge[dev] ? 1 : 0, - &sonic)) < 0) { + + err = snd_sonicvibes_create(card, pci, reverb[dev] ? 1 : 0, + mge[dev] ? 1 : 0, &sonic); + if (err < 0) { snd_card_free(card); return err; } @@ -1487,37 +1498,42 @@ static int snd_sonic_probe(struct pci_dev *pci, (unsigned long long)pci_resource_start(pci, 1), sonic->irq);
- if ((err = snd_sonicvibes_pcm(sonic, 0)) < 0) { + err = snd_sonicvibes_pcm(sonic, 0); + if (err < 0) { snd_card_free(card); return err; } - if ((err = snd_sonicvibes_mixer(sonic)) < 0) { + err = snd_sonicvibes_mixer(sonic); + if (err < 0) { snd_card_free(card); return err; } - if ((err = snd_mpu401_uart_new(card, 0, MPU401_HW_SONICVIBES, - sonic->midi_port, - MPU401_INFO_INTEGRATED | - MPU401_INFO_IRQ_HOOK, - -1, &midi_uart)) < 0) { + err = snd_mpu401_uart_new(card, 0, MPU401_HW_SONICVIBES, + sonic->midi_port, + MPU401_INFO_INTEGRATED | MPU401_INFO_IRQ_HOOK, + -1, &midi_uart); + if (err < 0) { snd_card_free(card); return err; } snd_sonicvibes_midi(sonic, midi_uart); - if ((err = snd_opl3_create(card, sonic->synth_port, - sonic->synth_port + 2, - OPL3_HW_OPL3_SV, 1, &opl3)) < 0) { + err = snd_opl3_create(card, sonic->synth_port, + sonic->synth_port + 2, + OPL3_HW_OPL3_SV, 1, &opl3); + if (err < 0) { snd_card_free(card); return err; } - if ((err = snd_opl3_hwdep_new(opl3, 0, 1, NULL)) < 0) { + err = snd_opl3_hwdep_new(opl3, 0, 1, NULL); + if (err < 0) { snd_card_free(card); return err; }
snd_sonicvibes_create_gameport(sonic);
- if ((err = snd_card_register(card)) < 0) { + err = snd_card_register(card); + if (err < 0) { snd_card_free(card); return err; }
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 18 Nov 2017 16:10:34 +0100
Add a jump target 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/sonicvibes.c | 54 ++++++++++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 30 deletions(-)
diff --git a/sound/pci/sonicvibes.c b/sound/pci/sonicvibes.c index 7c238d908031..75fc64839df0 100644 --- a/sound/pci/sonicvibes.c +++ b/sound/pci/sonicvibes.c @@ -1478,17 +1478,15 @@ static int snd_sonic_probe(struct pci_dev *pci, for (idx = 0; idx < 5; idx++) { if (pci_resource_start(pci, idx) == 0 || !(pci_resource_flags(pci, idx) & IORESOURCE_IO)) { - snd_card_free(card); - return -ENODEV; + err = -ENODEV; + goto free_card; } }
err = snd_sonicvibes_create(card, pci, reverb[dev] ? 1 : 0, mge[dev] ? 1 : 0, &sonic); - if (err < 0) { - snd_card_free(card); - return err; - } + if (err < 0) + goto free_card;
strcpy(card->driver, "SonicVibes"); strcpy(card->shortname, "S3 SonicVibes"); @@ -1499,48 +1497,44 @@ static int snd_sonic_probe(struct pci_dev *pci, sonic->irq);
err = snd_sonicvibes_pcm(sonic, 0); - if (err < 0) { - snd_card_free(card); - return err; - } + if (err < 0) + goto free_card; + err = snd_sonicvibes_mixer(sonic); - if (err < 0) { - snd_card_free(card); - return err; - } + if (err < 0) + goto free_card; + err = snd_mpu401_uart_new(card, 0, MPU401_HW_SONICVIBES, sonic->midi_port, MPU401_INFO_INTEGRATED | MPU401_INFO_IRQ_HOOK, -1, &midi_uart); - if (err < 0) { - snd_card_free(card); - return err; - } + if (err < 0) + goto free_card; + snd_sonicvibes_midi(sonic, midi_uart); err = snd_opl3_create(card, sonic->synth_port, sonic->synth_port + 2, OPL3_HW_OPL3_SV, 1, &opl3); - if (err < 0) { - snd_card_free(card); - return err; - } + if (err < 0) + goto free_card; + err = snd_opl3_hwdep_new(opl3, 0, 1, NULL); - if (err < 0) { - snd_card_free(card); - return err; - } + if (err < 0) + goto free_card;
snd_sonicvibes_create_gameport(sonic);
err = snd_card_register(card); - if (err < 0) { - snd_card_free(card); - return err; - } + if (err < 0) + goto free_card; pci_set_drvdata(pci, card); dev++; return 0; + +free_card: + snd_card_free(card); + return err; }
static void snd_sonic_remove(struct pci_dev *pci)
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 18 Nov 2017 16:12:12 +0100
Add jump targets so that a bit of exception handling can be better reused at the end of this function.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/pci/sonicvibes.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/sound/pci/sonicvibes.c b/sound/pci/sonicvibes.c index 75fc64839df0..c2147484c95d 100644 --- a/sound/pci/sonicvibes.c +++ b/sound/pci/sonicvibes.c @@ -1266,14 +1266,14 @@ static int snd_sonicvibes_create(struct snd_card *card, dma_set_coherent_mask(&pci->dev, DMA_BIT_MASK(24)) < 0) { dev_err(card->dev, "architecture does not support 24bit PCI busmaster DMA\n"); - pci_disable_device(pci); - return -ENXIO; + err = -ENXIO; + goto disable_device; }
sonic = kzalloc(sizeof(*sonic), GFP_KERNEL); if (sonic == NULL) { - pci_disable_device(pci); - return -ENOMEM; + err = -ENOMEM; + goto disable_device; } spin_lock_init(&sonic->reg_lock); sonic->card = card; @@ -1283,8 +1283,7 @@ static int snd_sonicvibes_create(struct snd_card *card, err = pci_request_regions(pci, "S3 SonicVibes"); if (err < 0) { kfree(sonic); - pci_disable_device(pci); - return err; + goto disable_device; }
sonic->sb_port = pci_resource_start(pci, 0); @@ -1296,8 +1295,7 @@ static int snd_sonicvibes_create(struct snd_card *card, if (request_irq(pci->irq, snd_sonicvibes_interrupt, IRQF_SHARED, KBUILD_MODNAME, sonic)) { dev_err(card->dev, "unable to grab IRQ %d\n", pci->irq); - snd_sonicvibes_free(sonic); - return -EBUSY; + goto e_busy; } sonic->irq = pci->irq;
@@ -1325,20 +1323,18 @@ static int snd_sonicvibes_create(struct snd_card *card,
sonic->res_dmaa = request_region(dmaa, 0x10, "S3 SonicVibes DDMA-A"); if (!sonic->res_dmaa) { - snd_sonicvibes_free(sonic); dev_err(card->dev, "unable to grab DDMA-A port at 0x%x-0x%x\n", dmaa, dmaa + 0x10 - 1); - return -EBUSY; + goto e_busy; }
sonic->res_dmac = request_region(dmac, 0x10, "S3 SonicVibes DDMA-C"); if (!sonic->res_dmac) { - snd_sonicvibes_free(sonic); dev_err(card->dev, "unable to grab DDMA-C port at 0x%x-0x%x\n", dmac, dmac + 0x10 - 1); - return -EBUSY; + goto e_busy; }
pci_read_config_dword(pci, 0x40, &sonic->dmaa_port); @@ -1395,15 +1391,23 @@ static int snd_sonicvibes_create(struct snd_card *card, #endif sonic->revision = snd_sonicvibes_in(sonic, SV_IREG_REVISION); err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, sonic, &ops); - if (err < 0) { - snd_sonicvibes_free(sonic); - return err; - } + if (err < 0) + goto free_sound_chip;
snd_sonicvibes_proc_init(sonic);
*rsonic = sonic; return 0; + +disable_device: + pci_disable_device(pci); + return err; + +e_busy: + err = -EBUSY; +free_sound_chip: + snd_sonicvibes_free(sonic); + return err; }
/*
participants (1)
-
SF Markus Elfring