[alsa-devel] [PATCH 0/4] ALSA-Riptide: Fine-tuning for seven function implementations
From: Markus Elfring elfring@users.sourceforge.net Date: Fri, 17 Nov 2017 10:32:23 +0100
A few update suggestions were taken into account from static source code analysis.
Markus Elfring (4): Adjust 13 function calls together with a variable assignment Delete an unnecessary variable initialisation in snd_riptide_proc_read() Reduce the scope for two variables in snd_riptide_proc_read() Use common error handling code in snd_riptide_create()
sound/pci/riptide/riptide.c | 78 ++++++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 32 deletions(-)
From: Markus Elfring elfring@users.sourceforge.net Date: Fri, 17 Nov 2017 09:43:55 +0100
* The script "checkpatch.pl" pointed information out like the following.
ERROR: do not use assignment in if condition
Thus fix affected source code places.
* Replace the specification of two data structures by pointer dereferences as the parameter for the operator "sizeof" to make the corresponding size determination a bit safer according to the Linux coding style convention.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/pci/riptide/riptide.c | 53 ++++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 20 deletions(-)
diff --git a/sound/pci/riptide/riptide.c b/sound/pci/riptide/riptide.c index 44f3b48d47b1..22d15ea7e210 100644 --- a/sound/pci/riptide/riptide.c +++ b/sound/pci/riptide/riptide.c @@ -1564,10 +1564,12 @@ snd_riptide_hw_params(struct snd_pcm_substream *substream, (int)sgdlist->bytes); if (sgdlist->area) snd_dma_free_pages(sgdlist); - if ((err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, - snd_dma_pci_data(chip->pci), - sizeof(struct sgd) * (DESC_MAX_MASK + 1), - sgdlist)) < 0) { + + err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, + snd_dma_pci_data(chip->pci), + sizeof(struct sgd) * (DESC_MAX_MASK + 1), + sgdlist); + if (err < 0) { snd_printk(KERN_ERR "Riptide: failed to alloc %d dma bytes\n", (int)sizeof(struct sgd) * (DESC_MAX_MASK + 1)); return err; @@ -1695,11 +1697,10 @@ static const struct snd_pcm_ops snd_riptide_capture_ops = { static int snd_riptide_pcm(struct snd_riptide *chip, int device) { struct snd_pcm *pcm; - int err; + int err = snd_pcm_new(chip->card, "RIPTIDE", device, + PLAYBACK_SUBSTREAMS, 1, &pcm);
- if ((err = - snd_pcm_new(chip->card, "RIPTIDE", device, PLAYBACK_SUBSTREAMS, 1, - &pcm)) < 0) + if (err < 0) return err; snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &snd_riptide_playback_ops); @@ -1786,14 +1787,16 @@ static int snd_riptide_initialize(struct snd_riptide *chip)
cif = chip->cif; if (!cif) { - if ((cif = kzalloc(sizeof(struct cmdif), GFP_KERNEL)) == NULL) + cif = kzalloc(sizeof(*cif), GFP_KERNEL); + if (!cif) return -ENOMEM; cif->hwport = (struct riptideport *)chip->port; spin_lock_init(&cif->lock); chip->cif = cif; } cif->is_reset = 0; - if ((err = riptide_reset(cif, chip)) != 0) + err = riptide_reset(cif, chip); + if (err) return err; device_id = chip->device_id; switch (device_id) { @@ -1817,7 +1820,8 @@ static int snd_riptide_free(struct snd_riptide *chip) if (!chip) return 0;
- if ((cif = chip->cif)) { + cif = chip->cif; + if (cif) { SET_GRESET(cif->hwport); udelay(100); UNSET_GRESET(cif->hwport); @@ -1850,9 +1854,12 @@ snd_riptide_create(struct snd_card *card, struct pci_dev *pci, };
*rchip = NULL; - if ((err = pci_enable_device(pci)) < 0) + err = pci_enable_device(pci); + if (err < 0) return err; - if (!(chip = kzalloc(sizeof(struct snd_riptide), GFP_KERNEL))) + + chip = kzalloc(sizeof(*chip), GFP_KERNEL); + if (!chip) return -ENOMEM;
spin_lock_init(&chip->lock); @@ -1866,8 +1873,8 @@ snd_riptide_create(struct snd_card *card, struct pci_dev *pci, chip->cif = NULL; tasklet_init(&chip->riptide_tq, riptide_handleirq, (unsigned long)chip);
- if ((chip->res_port = - request_region(chip->port, 64, "RIPTIDE")) == NULL) { + chip->res_port = request_region(chip->port, 64, "RIPTIDE"); + if (!chip->res_port) { snd_printk(KERN_ERR "Riptide: unable to grab region 0x%lx-0x%lx\n", chip->port, chip->port + 64 - 1); @@ -1887,12 +1894,14 @@ snd_riptide_create(struct snd_card *card, struct pci_dev *pci, chip->irq = pci->irq; chip->device_id = pci->device; pci_set_master(pci); - if ((err = snd_riptide_initialize(chip)) < 0) { + err = snd_riptide_initialize(chip); + if (err < 0) { snd_riptide_free(chip); return err; }
- 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_riptide_free(chip); return err; } @@ -1922,7 +1931,9 @@ snd_riptide_proc_read(struct snd_info_entry *entry, for (i = 0; i < 64; i += 4) snd_iprintf(buffer, "%c%02x: %08x", (i % 16) ? ' ' : '\n', i, inl(chip->port + i)); - if ((cif = chip->cif)) { + + cif = chip->cif; + if (cif) { snd_iprintf(buffer, "\nVersion: ASIC: %d CODEC: %d AUXDSP: %d PROG: %d", chip->firmware.firmware.ASIC, @@ -1994,12 +2005,14 @@ static int snd_riptide_mixer(struct snd_riptide *chip) ac97.private_data = chip; ac97.scaps = AC97_SCAP_SKIP_MODEM;
- if ((err = snd_ac97_bus(chip->card, 0, &ops, chip, &pbus)) < 0) + err = snd_ac97_bus(chip->card, 0, &ops, chip, &pbus); + if (err < 0) return err;
chip->ac97_bus = pbus; ac97.pci = chip->pci; - if ((err = snd_ac97_mixer(pbus, &ac97, &chip->ac97)) < 0) + err = snd_ac97_mixer(pbus, &ac97, &chip->ac97); + if (err < 0) return err; return err; }
From: Markus Elfring elfring@users.sourceforge.net Date: Fri, 17 Nov 2017 09:50:52 +0100
The variable "cif" will be set to an appropriate pointer a bit later. Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/pci/riptide/riptide.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/riptide/riptide.c b/sound/pci/riptide/riptide.c index 22d15ea7e210..3d00488ff137 100644 --- a/sound/pci/riptide/riptide.c +++ b/sound/pci/riptide/riptide.c @@ -1917,5 +1917,5 @@ snd_riptide_proc_read(struct snd_info_entry *entry, struct snd_riptide *chip = entry->private_data; struct pcmhw *data; int i; - struct cmdif *cif = NULL; + struct cmdif *cif; unsigned char p[256];
From: Markus Elfring elfring@users.sourceforge.net Date: Fri, 17 Nov 2017 09:55:27 +0100
Move the definitions for the local variables "lval" and "rval" into an if branch so that the corresponding setting will only be performed on concrete demand in this function.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/pci/riptide/riptide.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/pci/riptide/riptide.c b/sound/pci/riptide/riptide.c index 3d00488ff137..774fba5bba48 100644 --- a/sound/pci/riptide/riptide.c +++ b/sound/pci/riptide/riptide.c @@ -1921,5 +1921,4 @@ snd_riptide_proc_read(struct snd_info_entry *entry, unsigned char p[256]; - unsigned short rval = 0, lval = 0; unsigned int rate;
if (!chip) @@ -1934,6 +1933,8 @@ snd_riptide_proc_read(struct snd_info_entry *entry,
cif = chip->cif; if (cif) { + unsigned short rval = 0, lval = 0; + snd_iprintf(buffer, "\nVersion: ASIC: %d CODEC: %d AUXDSP: %d PROG: %d", chip->firmware.firmware.ASIC,
From: Markus Elfring elfring@users.sourceforge.net Date: Fri, 17 Nov 2017 10:22:33 +0100
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/riptide/riptide.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/sound/pci/riptide/riptide.c b/sound/pci/riptide/riptide.c index 774fba5bba48..b1b834e1b614 100644 --- a/sound/pci/riptide/riptide.c +++ b/sound/pci/riptide/riptide.c @@ -1878,8 +1878,7 @@ snd_riptide_create(struct snd_card *card, struct pci_dev *pci, snd_printk(KERN_ERR "Riptide: unable to grab region 0x%lx-0x%lx\n", chip->port, chip->port + 64 - 1); - snd_riptide_free(chip); - return -EBUSY; + goto e_busy; } hwport = (struct riptideport *)chip->port; UNSET_AIE(hwport); @@ -1888,26 +1887,27 @@ snd_riptide_create(struct snd_card *card, struct pci_dev *pci, KBUILD_MODNAME, chip)) { snd_printk(KERN_ERR "Riptide: unable to grab IRQ %d\n", pci->irq); - snd_riptide_free(chip); - return -EBUSY; + goto e_busy; } chip->irq = pci->irq; chip->device_id = pci->device; pci_set_master(pci); err = snd_riptide_initialize(chip); - if (err < 0) { - snd_riptide_free(chip); - return err; - } + if (err < 0) + goto free_sound_chip;
err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops); - if (err < 0) { - snd_riptide_free(chip); - return err; - } + if (err < 0) + goto free_sound_chip;
*rchip = chip; return 0; + +e_busy: + err = -EBUSY; +free_sound_chip: + snd_riptide_free(chip); + return err; }
static void
participants (1)
-
SF Markus Elfring