[alsa-devel] [PATCH 0/3] ALSA: cs4281: Fine-tuning for five function implementations
From: Markus Elfring elfring@users.sourceforge.net Date: Mon, 13 Nov 2017 13:46:23 +0100
A few update suggestions were taken into account from static source code analysis.
Markus Elfring (3): Adjust 18 function calls together with a variable assignment Use common error handling code in snd_cs4281_probe() Use common error handling code in snd_cs4281_create()
sound/pci/cs4281.c | 136 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 77 insertions(+), 59 deletions(-)
From: Markus Elfring elfring@users.sourceforge.net Date: Mon, 13 Nov 2017 12:52:53 +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/cs4281.c | 59 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 19 deletions(-)
diff --git a/sound/pci/cs4281.c b/sound/pci/cs4281.c index ec4247638fa1..cf0bcab2a36b 100644 --- a/sound/pci/cs4281.c +++ b/sound/pci/cs4281.c @@ -1102,23 +1102,31 @@ static int snd_cs4281_mixer(struct cs4281 *chip) .read = snd_cs4281_ac97_read, };
- if ((err = snd_ac97_bus(card, 0, &ops, chip, &chip->ac97_bus)) < 0) + err = snd_ac97_bus(card, 0, &ops, chip, &chip->ac97_bus); + if (err < 0) return err; chip->ac97_bus->private_free = snd_cs4281_mixer_free_ac97_bus;
memset(&ac97, 0, sizeof(ac97)); ac97.private_data = chip; ac97.private_free = snd_cs4281_mixer_free_ac97; - if ((err = snd_ac97_mixer(chip->ac97_bus, &ac97, &chip->ac97)) < 0) + err = snd_ac97_mixer(chip->ac97_bus, &ac97, &chip->ac97); + if (err < 0) return err; if (chip->dual_codec) { ac97.num = 1; - if ((err = snd_ac97_mixer(chip->ac97_bus, &ac97, &chip->ac97_secondary)) < 0) + err = snd_ac97_mixer(chip->ac97_bus, &ac97, + &chip->ac97_secondary); + if (err < 0) return err; } - if ((err = snd_ctl_add(card, snd_ctl_new1(&snd_cs4281_fm_vol, chip))) < 0) + + err = snd_ctl_add(card, snd_ctl_new1(&snd_cs4281_fm_vol, chip)); + if (err < 0) return err; - if ((err = snd_ctl_add(card, snd_ctl_new1(&snd_cs4281_pcm_vol, chip))) < 0) + + err = snd_ctl_add(card, snd_ctl_new1(&snd_cs4281_pcm_vol, chip)); + if (err < 0) return err; return 0; } @@ -1346,7 +1354,8 @@ static int snd_cs4281_create(struct snd_card *card, };
*rchip = NULL; - if ((err = pci_enable_device(pci)) < 0) + err = pci_enable_device(pci); + if (err < 0) return err; chip = kzalloc(sizeof(*chip), GFP_KERNEL); if (chip == NULL) { @@ -1364,7 +1373,8 @@ static int snd_cs4281_create(struct snd_card *card, } chip->dual_codec = dual_codec;
- if ((err = pci_request_regions(pci, "CS4281")) < 0) { + err = pci_request_regions(pci, "CS4281"); + if (err < 0) { kfree(chip); pci_disable_device(pci); return err; @@ -1393,7 +1403,8 @@ static int snd_cs4281_create(struct snd_card *card, return tmp; }
- if ((err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops)) < 0) { + err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops); + if (err < 0) { snd_cs4281_free(chip); return err; } @@ -1432,12 +1443,15 @@ static int snd_cs4281_chip_init(struct cs4281 *chip) * space between 0e4h and 0ffh to be written. */ snd_cs4281_pokeBA0(chip, BA0_CWPR, 0x4281); - if ((tmp = snd_cs4281_peekBA0(chip, BA0_SERC1)) != (BA0_SERC1_SO1EN | BA0_SERC1_AC97)) { + tmp = snd_cs4281_peekBA0(chip, BA0_SERC1); + if (tmp != (BA0_SERC1_SO1EN | BA0_SERC1_AC97)) { dev_err(chip->card->dev, "SERC1 AC'97 check failed (0x%x)\n", tmp); return -EIO; } - if ((tmp = snd_cs4281_peekBA0(chip, BA0_SERC2)) != (BA0_SERC2_SI1EN | BA0_SERC2_AC97)) { + + tmp = snd_cs4281_peekBA0(chip, BA0_SERC2); + if (tmp != (BA0_SERC2_SI1EN | BA0_SERC2_AC97)) { dev_err(chip->card->dev, "SERC2 AC'97 check failed (0x%x)\n", tmp); return -EIO; @@ -1784,9 +1798,9 @@ static const struct snd_rawmidi_ops snd_cs4281_midi_input = static int snd_cs4281_midi(struct cs4281 *chip, int device) { struct snd_rawmidi *rmidi; - int err; + int err = snd_rawmidi_new(chip->card, "CS4281", device, 1, 1, &rmidi);
- if ((err = snd_rawmidi_new(chip->card, "CS4281", device, 1, 1, &rmidi)) < 0) + if (err < 0) return err; strcpy(rmidi->name, "CS4281"); snd_rawmidi_set_ops(rmidi, SNDRV_RAWMIDI_STREAM_OUTPUT, &snd_cs4281_midi_output); @@ -1919,32 +1933,38 @@ static int snd_cs4281_probe(struct pci_dev *pci, if (err < 0) return err;
- if ((err = snd_cs4281_create(card, pci, &chip, dual_codec[dev])) < 0) { + err = snd_cs4281_create(card, pci, &chip, dual_codec[dev]); + if (err < 0) { snd_card_free(card); return err; } card->private_data = chip;
- if ((err = snd_cs4281_mixer(chip)) < 0) { + err = snd_cs4281_mixer(chip); + if (err < 0) { snd_card_free(card); return err; } - if ((err = snd_cs4281_pcm(chip, 0)) < 0) { + err = snd_cs4281_pcm(chip, 0); + if (err < 0) { snd_card_free(card); return err; } - if ((err = snd_cs4281_midi(chip, 0)) < 0) { + err = snd_cs4281_midi(chip, 0); + if (err < 0) { snd_card_free(card); return err; } - if ((err = snd_opl3_new(card, OPL3_HW_OPL3_CS4281, &opl3)) < 0) { + err = snd_opl3_new(card, OPL3_HW_OPL3_CS4281, &opl3); + if (err < 0) { snd_card_free(card); return err; } opl3->private_data = chip; opl3->command = snd_cs4281_opl3_command; snd_opl3_init(opl3); - 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; } @@ -1956,7 +1976,8 @@ static int snd_cs4281_probe(struct pci_dev *pci, chip->ba0_addr, chip->irq);
- 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: Mon, 13 Nov 2017 13:10:11 +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/cs4281.c | 52 ++++++++++++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 28 deletions(-)
diff --git a/sound/pci/cs4281.c b/sound/pci/cs4281.c index cf0bcab2a36b..ed6b97dbd1cc 100644 --- a/sound/pci/cs4281.c +++ b/sound/pci/cs4281.c @@ -1934,40 +1934,34 @@ static int snd_cs4281_probe(struct pci_dev *pci, return err;
err = snd_cs4281_create(card, pci, &chip, dual_codec[dev]); - if (err < 0) { - snd_card_free(card); - return err; - } + if (err < 0) + goto free_card; + card->private_data = chip;
err = snd_cs4281_mixer(chip); - if (err < 0) { - snd_card_free(card); - return err; - } + if (err < 0) + goto free_card; + err = snd_cs4281_pcm(chip, 0); - if (err < 0) { - snd_card_free(card); - return err; - } + if (err < 0) + goto free_card; + err = snd_cs4281_midi(chip, 0); - if (err < 0) { - snd_card_free(card); - return err; - } + if (err < 0) + goto free_card; + err = snd_opl3_new(card, OPL3_HW_OPL3_CS4281, &opl3); - if (err < 0) { - snd_card_free(card); - return err; - } + if (err < 0) + goto free_card; + opl3->private_data = chip; opl3->command = snd_cs4281_opl3_command; snd_opl3_init(opl3); 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_cs4281_create_gameport(chip); strcpy(card->driver, "CS4281"); strcpy(card->shortname, "Cirrus Logic CS4281"); @@ -1977,14 +1971,16 @@ static int snd_cs4281_probe(struct pci_dev *pci, chip->irq);
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_cs4281_remove(struct pci_dev *pci)
From: Markus Elfring elfring@users.sourceforge.net Date: Mon, 13 Nov 2017 13:34:57 +0100
* Add jump targets so that a bit of exception handling can be better reused at the end of this function.
* Delete the local variable "tmp" which was unnecessary because the variable "err" could be used at this place again.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/pci/cs4281.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-)
diff --git a/sound/pci/cs4281.c b/sound/pci/cs4281.c index ed6b97dbd1cc..6cd7300a0182 100644 --- a/sound/pci/cs4281.c +++ b/sound/pci/cs4281.c @@ -1347,7 +1347,6 @@ static int snd_cs4281_create(struct snd_card *card, int dual_codec) { struct cs4281 *chip; - unsigned int tmp; int err; static struct snd_device_ops ops = { .dev_free = snd_cs4281_dev_free, @@ -1359,8 +1358,8 @@ static int snd_cs4281_create(struct snd_card *card, return err; chip = kzalloc(sizeof(*chip), GFP_KERNEL); if (chip == NULL) { - pci_disable_device(pci); - return -ENOMEM; + err = -ENOMEM; + goto disable_device; } spin_lock_init(&chip->reg_lock); chip->card = card; @@ -1376,43 +1375,45 @@ static int snd_cs4281_create(struct snd_card *card, err = pci_request_regions(pci, "CS4281"); if (err < 0) { kfree(chip); - pci_disable_device(pci); - return err; + goto disable_device; } chip->ba0_addr = pci_resource_start(pci, 0); chip->ba1_addr = pci_resource_start(pci, 1);
chip->ba0 = pci_ioremap_bar(pci, 0); chip->ba1 = pci_ioremap_bar(pci, 1); - if (!chip->ba0 || !chip->ba1) { - snd_cs4281_free(chip); - return -ENOMEM; - } + if (!chip->ba0 || !chip->ba1) + goto e_nomem; if (request_irq(pci->irq, snd_cs4281_interrupt, IRQF_SHARED, KBUILD_MODNAME, chip)) { dev_err(card->dev, "unable to grab IRQ %d\n", pci->irq); - snd_cs4281_free(chip); - return -ENOMEM; + goto e_nomem; } chip->irq = pci->irq;
- tmp = snd_cs4281_chip_init(chip); - if (tmp) { - snd_cs4281_free(chip); - return tmp; - } + err = snd_cs4281_chip_init(chip); + if (err) + goto free_sound_chip;
err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops); - if (err < 0) { - snd_cs4281_free(chip); - return err; - } + if (err < 0) + goto free_sound_chip;
snd_cs4281_proc_init(chip);
*rchip = chip; return 0; + +disable_device: + pci_disable_device(pci); + return err; + +e_nomem: + err = -ENOMEM; +free_sound_chip: + snd_cs4281_free(chip); + return err; }
static int snd_cs4281_chip_init(struct cs4281 *chip)
participants (1)
-
SF Markus Elfring