[PATCH 0/4] ALSA-Riptide: Fine-tuning for seven function implementations

[PATCH 0/4] ALSA-Riptide: Fine-tuning for seven function implementations

[SF Markus Elfring]

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(-)

-- 
2.15.0

[PATCH 1/4] ALSA: riptide: Adjust 13 function calls together with a variable assignment

[SF Markus Elfring]

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;
 }
-- 
2.15.0

[PATCH 2/4] ALSA: riptide: Delete an unnecessary variable initialisation in snd_riptide_proc_read()

[SF Markus Elfring]

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];
-- 
2.15.0

[PATCH 3/4] ALSA: riptide: Reduce the scope for two variables in snd_riptide_proc_read()

[SF Markus Elfring]

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,
-- 
2.15.0

[PATCH 4/4] ALSA: riptide: Use common error handling code in snd_riptide_create()

[SF Markus Elfring]

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
-- 
2.15.0