[PATCH 0/4] ALSA: cs46xx: Adjustments for some function implementations

[PATCH 0/4] ALSA: cs46xx: Adjustments for some function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 13 Nov 2017 19:14:56 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (4):
  Adjust 33 function calls together with a variable assignment
  Use common error handling code in two functions
  Improve a size determination in cs46xx_dsp_proc_register_scb_desc()
  Adjust 35 checks for null pointers

 sound/pci/cs46xx/cs46xx.c           |  75 ++++++++++----------
 sound/pci/cs46xx/cs46xx_lib.c       | 134 +++++++++++++++++++++---------------
 sound/pci/cs46xx/dsp_spos.c         |  60 +++++++++-------
 sound/pci/cs46xx/dsp_spos_scb_lib.c |  36 +++++-----
 4 files changed, 166 insertions(+), 139 deletions(-)

-- 
2.15.0

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

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 13 Nov 2017 18:50:19 +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/cs46xx/cs46xx.c           | 31 ++++++++++------
 sound/pci/cs46xx/cs46xx_lib.c       | 71 ++++++++++++++++++++++++-------------
 sound/pci/cs46xx/dsp_spos.c         | 29 ++++++++++-----
 sound/pci/cs46xx/dsp_spos_scb_lib.c |  7 ++--
 4 files changed, 92 insertions(+), 46 deletions(-)

diff --git a/sound/pci/cs46xx/cs46xx.c b/sound/pci/cs46xx/cs46xx.c
index 655fbea1692c..520478a6e605 100644
--- a/sound/pci/cs46xx/cs46xx.c
+++ b/sound/pci/cs46xx/cs46xx.c
@@ -92,45 +92,53 @@ static int snd_card_cs46xx_probe(struct pci_dev *pci,
 			   0, &card);
 	if (err < 0)
 		return err;
-	if ((err = snd_cs46xx_create(card, pci,
-				     external_amp[dev], thinkpad[dev],
-				     &chip)) < 0) {
+
+	err = snd_cs46xx_create(card, pci, external_amp[dev],
+				thinkpad[dev], &chip);
+	if (err < 0) {
 		snd_card_free(card);
 		return err;
 	}
 	card->private_data = chip;
 	chip->accept_valid = mmap_valid[dev];
-	if ((err = snd_cs46xx_pcm(chip, 0)) < 0) {
+	err = snd_cs46xx_pcm(chip, 0);
+	if (err < 0) {
 		snd_card_free(card);
 		return err;
 	}
 #ifdef CONFIG_SND_CS46XX_NEW_DSP
-	if ((err = snd_cs46xx_pcm_rear(chip, 1)) < 0) {
+	err = snd_cs46xx_pcm_rear(chip, 1);
+	if (err < 0) {
 		snd_card_free(card);
 		return err;
 	}
-	if ((err = snd_cs46xx_pcm_iec958(chip, 2)) < 0) {
+	err = snd_cs46xx_pcm_iec958(chip, 2);
+	if (err < 0) {
 		snd_card_free(card);
 		return err;
 	}
 #endif
-	if ((err = snd_cs46xx_mixer(chip, 2)) < 0) {
+	err = snd_cs46xx_mixer(chip, 2);
+	if (err < 0) {
 		snd_card_free(card);
 		return err;
 	}
 #ifdef CONFIG_SND_CS46XX_NEW_DSP
 	if (chip->nr_ac97_codecs ==2) {
-		if ((err = snd_cs46xx_pcm_center_lfe(chip, 3)) < 0) {
+		err = snd_cs46xx_pcm_center_lfe(chip, 3);
+		if (err < 0) {
 			snd_card_free(card);
 			return err;
 		}
 	}
 #endif
-	if ((err = snd_cs46xx_midi(chip, 0)) < 0) {
+	err = snd_cs46xx_midi(chip, 0);
+	if (err < 0) {
 		snd_card_free(card);
 		return err;
 	}
-	if ((err = snd_cs46xx_start_dsp(chip)) < 0) {
+	err = snd_cs46xx_start_dsp(chip);
+	if (err < 0) {
 		snd_card_free(card);
 		return err;
 	}
@@ -146,7 +154,8 @@ static int snd_card_cs46xx_probe(struct pci_dev *pci,
 		chip->ba1_addr,
 		chip->irq);
 
-	if ((err = snd_card_register(card)) < 0) {
+	err = snd_card_register(card);
+	if (err < 0) {
 		snd_card_free(card);
 		return err;
 	}
diff --git a/sound/pci/cs46xx/cs46xx_lib.c b/sound/pci/cs46xx/cs46xx_lib.c
index 0020fd0efc46..164f86949b2d 100644
--- a/sound/pci/cs46xx/cs46xx_lib.c
+++ b/sound/pci/cs46xx/cs46xx_lib.c
@@ -1071,10 +1071,14 @@ static int _cs46xx_adjust_sample_rate (struct snd_cs46xx *chip, struct snd_cs46x
 	if ((int)cpcm->pcm_channel->sample_rate != sample_rate) {
 		int unlinked = cpcm->pcm_channel->unlinked;
 		cs46xx_dsp_destroy_pcm_channel (chip,cpcm->pcm_channel);
-
-		if ( (cpcm->pcm_channel = cs46xx_dsp_create_pcm_channel (chip, sample_rate, cpcm, 
-									 cpcm->hw_buf.addr,
-									 cpcm->pcm_channel_id)) == NULL) {
+		cpcm
+		->pcm_channel = cs46xx_dsp_create_pcm_channel(chip,
+							      sample_rate,
+							      cpcm,
+							      cpcm->hw_buf.addr,
+							      cpcm
+							      ->pcm_channel_id);
+		if (!cpcm->pcm_channel) {
 			dev_err(chip->card->dev,
 				"failed to re-create virtual PCM channel\n");
 			return -ENOMEM;
@@ -1161,7 +1165,10 @@ static int snd_cs46xx_playback_hw_params(struct snd_pcm_substream *substream,
 			runtime->dma_addr = 0;
 			runtime->dma_bytes = 0;
 		}
-		if ((err = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(hw_params))) < 0) {
+
+		err = snd_pcm_lib_malloc_pages(substream,
+					       params_buffer_bytes(hw_params));
+		if (err < 0) {
 #ifdef CONFIG_SND_CS46XX_NEW_DSP
 			mutex_unlock(&chip->spos_mutex);
 #endif
@@ -1309,7 +1316,10 @@ static int snd_cs46xx_capture_hw_params(struct snd_pcm_substream *substream,
 			runtime->dma_addr = 0;
 			runtime->dma_bytes = 0;
 		}
-		if ((err = snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(hw_params))) < 0)
+
+		err = snd_pcm_lib_malloc_pages(substream,
+					       params_buffer_bytes(hw_params));
+		if (err < 0)
 			return err;
 		substream->ops = &snd_cs46xx_capture_indirect_ops;
 	}
@@ -1780,9 +1790,10 @@ static const struct snd_pcm_ops snd_cs46xx_capture_indirect_ops = {
 int snd_cs46xx_pcm(struct snd_cs46xx *chip, int device)
 {
 	struct snd_pcm *pcm;
-	int err;
+	int err = snd_pcm_new(chip->card, "CS46xx", device,
+			      MAX_PLAYBACK_CHANNELS, 1, &pcm);
 
-	if ((err = snd_pcm_new(chip->card, "CS46xx", device, MAX_PLAYBACK_CHANNELS, 1, &pcm)) < 0)
+	if (err < 0)
 		return err;
 
 	pcm->private_data = chip;
@@ -1806,9 +1817,10 @@ int snd_cs46xx_pcm(struct snd_cs46xx *chip, int device)
 int snd_cs46xx_pcm_rear(struct snd_cs46xx *chip, int device)
 {
 	struct snd_pcm *pcm;
-	int err;
+	int err = snd_pcm_new(chip->card, "CS46xx - Rear",
+			      device, MAX_PLAYBACK_CHANNELS, 0, &pcm);
 
-	if ((err = snd_pcm_new(chip->card, "CS46xx - Rear", device, MAX_PLAYBACK_CHANNELS, 0, &pcm)) < 0)
+	if (err < 0)
 		return err;
 
 	pcm->private_data = chip;
@@ -1829,9 +1841,10 @@ int snd_cs46xx_pcm_rear(struct snd_cs46xx *chip, int device)
 int snd_cs46xx_pcm_center_lfe(struct snd_cs46xx *chip, int device)
 {
 	struct snd_pcm *pcm;
-	int err;
+	int err = snd_pcm_new(chip->card, "CS46xx - Center LFE",
+			      device, MAX_PLAYBACK_CHANNELS, 0, &pcm);
 
-	if ((err = snd_pcm_new(chip->card, "CS46xx - Center LFE", device, MAX_PLAYBACK_CHANNELS, 0, &pcm)) < 0)
+	if (err < 0)
 		return err;
 
 	pcm->private_data = chip;
@@ -1852,9 +1865,10 @@ int snd_cs46xx_pcm_center_lfe(struct snd_cs46xx *chip, int device)
 int snd_cs46xx_pcm_iec958(struct snd_cs46xx *chip, int device)
 {
 	struct snd_pcm *pcm;
-	int err;
+	int err = snd_pcm_new(chip->card, "CS46xx - IEC958",
+			      device, 1, 0, &pcm);
 
-	if ((err = snd_pcm_new(chip->card, "CS46xx - IEC958", device, 1, 0, &pcm)) < 0)
+	if (err < 0)
 		return err;
 
 	pcm->private_data = chip;
@@ -2432,7 +2446,8 @@ static void snd_cs46xx_codec_reset (struct snd_ac97 * ac97)
 
 		/* test if we can write to the record gain volume register */
 		snd_ac97_write(ac97, AC97_REC_GAIN, 0x8a05);
-		if ((err = snd_ac97_read(ac97, AC97_REC_GAIN)) == 0x8a05)
+		err = snd_ac97_read(ac97, AC97_REC_GAIN);
+		if (err == 0x8a05)
 			return;
 
 		msleep(10);
@@ -2494,7 +2509,8 @@ int snd_cs46xx_mixer(struct snd_cs46xx *chip, int spdif_device)
 	/* detect primary codec */
 	chip->nr_ac97_codecs = 0;
 	dev_dbg(chip->card->dev, "detecting primary codec\n");
-	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_cs46xx_mixer_free_ac97_bus;
 
@@ -2515,7 +2531,9 @@ int snd_cs46xx_mixer(struct snd_cs46xx *chip, int spdif_device)
 		kctl = snd_ctl_new1(&snd_cs46xx_controls[idx], chip);
 		if (kctl && kctl->id.iface == SNDRV_CTL_ELEM_IFACE_PCM)
 			kctl->id.device = spdif_device;
-		if ((err = snd_ctl_add(card, kctl)) < 0)
+
+		err = snd_ctl_add(card, kctl);
+		if (err < 0)
 			return err;
 	}
 
@@ -2700,9 +2718,9 @@ static const struct snd_rawmidi_ops snd_cs46xx_midi_input =
 int snd_cs46xx_midi(struct snd_cs46xx *chip, int device)
 {
 	struct snd_rawmidi *rmidi;
-	int err;
+	int err = snd_rawmidi_new(chip->card, "CS46XX", device, 1, 1, &rmidi);
 
-	if ((err = snd_rawmidi_new(chip->card, "CS46XX", device, 1, 1, &rmidi)) < 0)
+	if (err < 0)
 		return err;
 	strcpy(rmidi->name, "CS46XX");
 	snd_rawmidi_set_ops(rmidi, SNDRV_RAWMIDI_STREAM_OUTPUT, &snd_cs46xx_midi_output);
@@ -3544,7 +3562,8 @@ static void hercules_mixer_init (struct snd_cs46xx *chip)
 		struct snd_kcontrol *kctl;
 
 		kctl = snd_ctl_new1(&snd_hercules_controls[idx], chip);
-		if ((err = snd_ctl_add(card, kctl)) < 0) {
+		err = snd_ctl_add(card, kctl);
+		if (err < 0) {
 			dev_err(card->dev,
 				"failed to initialize Hercules mixer (%d)\n",
 				err);
@@ -3895,7 +3914,8 @@ int snd_cs46xx_create(struct snd_card *card,
 	*rchip = NULL;
 
 	/* enable PCI device */
-	if ((err = pci_enable_device(pci)) < 0)
+	err = pci_enable_device(pci);
+	if (err < 0)
 		return err;
 
 	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
@@ -3989,8 +4009,10 @@ int snd_cs46xx_create(struct snd_card *card,
 
 	for (idx = 0; idx < 5; idx++) {
 		region = &chip->region.idx[idx];
-		if ((region->resource = request_mem_region(region->base, region->size,
-							   region->name)) == NULL) {
+		region->resource = request_mem_region(region->base,
+						      region->size,
+						      region->name);
+		if (!region->resource) {
 			dev_err(chip->card->dev,
 				"unable to request memory region 0x%lx-0x%lx\n",
 				   region->base, region->base + region->size - 1);
@@ -4028,7 +4050,8 @@ int snd_cs46xx_create(struct snd_card *card,
 		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_cs46xx_free(chip);
 		return err;
 	}
diff --git a/sound/pci/cs46xx/dsp_spos.c b/sound/pci/cs46xx/dsp_spos.c
index aa61615288ff..c30bb557468c 100644
--- a/sound/pci/cs46xx/dsp_spos.c
+++ b/sound/pci/cs46xx/dsp_spos.c
@@ -627,7 +627,9 @@ static void cs46xx_dsp_proc_parameter_dump_read (struct snd_info_entry *entry,
 			col = 0;
 		}
 
-		if ( (symbol = cs46xx_dsp_lookup_symbol_addr (chip,i / sizeof(u32), SYMBOL_PARAMETER)) != NULL) {
+		symbol = cs46xx_dsp_lookup_symbol_addr(chip, i / sizeof(u32),
+						       SYMBOL_PARAMETER);
+		if (symbol) {
 			col = 0;
 			snd_iprintf (buffer,"\n%s:\n",symbol->symbol_name);
 		}
@@ -796,7 +798,8 @@ int cs46xx_dsp_proc_init (struct snd_card *card, struct snd_cs46xx *chip)
 
 	ins->snd_card = card;
 
-	if ((entry = snd_info_create_card_entry(card, "dsp", card->proc_root)) != NULL) {
+	entry = snd_info_create_card_entry(card, "dsp", card->proc_root);
+	if (entry) {
 		entry->content = SNDRV_INFO_CONTENT_TEXT;
 		entry->mode = S_IFDIR | S_IRUGO | S_IXUGO;
       
@@ -811,7 +814,9 @@ int cs46xx_dsp_proc_init (struct snd_card *card, struct snd_cs46xx *chip)
 	if (!ins->proc_dsp_dir)
 		return -ENOMEM;
 
-	if ((entry = snd_info_create_card_entry(card, "spos_symbols", ins->proc_dsp_dir)) != NULL) {
+	entry = snd_info_create_card_entry(card, "spos_symbols",
+					   ins->proc_dsp_dir);
+	if (entry) {
 		entry->content = SNDRV_INFO_CONTENT_TEXT;
 		entry->private_data = chip;
 		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
@@ -823,7 +828,9 @@ int cs46xx_dsp_proc_init (struct snd_card *card, struct snd_cs46xx *chip)
 	}
 	ins->proc_sym_info_entry = entry;
     
-	if ((entry = snd_info_create_card_entry(card, "spos_modules", ins->proc_dsp_dir)) != NULL) {
+	entry = snd_info_create_card_entry(card, "spos_modules",
+					   ins->proc_dsp_dir);
+	if (entry) {
 		entry->content = SNDRV_INFO_CONTENT_TEXT;
 		entry->private_data = chip;
 		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
@@ -835,7 +842,9 @@ int cs46xx_dsp_proc_init (struct snd_card *card, struct snd_cs46xx *chip)
 	}
 	ins->proc_modules_info_entry = entry;
 
-	if ((entry = snd_info_create_card_entry(card, "parameter", ins->proc_dsp_dir)) != NULL) {
+	entry = snd_info_create_card_entry(card, "parameter",
+					   ins->proc_dsp_dir);
+	if (entry) {
 		entry->content = SNDRV_INFO_CONTENT_TEXT;
 		entry->private_data = chip;
 		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
@@ -847,7 +856,8 @@ int cs46xx_dsp_proc_init (struct snd_card *card, struct snd_cs46xx *chip)
 	}
 	ins->proc_parameter_dump_info_entry = entry;
 
-	if ((entry = snd_info_create_card_entry(card, "sample", ins->proc_dsp_dir)) != NULL) {
+	entry = snd_info_create_card_entry(card, "sample", ins->proc_dsp_dir);
+	if (entry) {
 		entry->content = SNDRV_INFO_CONTENT_TEXT;
 		entry->private_data = chip;
 		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
@@ -859,7 +869,9 @@ int cs46xx_dsp_proc_init (struct snd_card *card, struct snd_cs46xx *chip)
 	}
 	ins->proc_sample_dump_info_entry = entry;
 
-	if ((entry = snd_info_create_card_entry(card, "task_tree", ins->proc_dsp_dir)) != NULL) {
+	entry = snd_info_create_card_entry(card, "task_tree",
+					   ins->proc_dsp_dir);
+	if (entry) {
 		entry->content = SNDRV_INFO_CONTENT_TEXT;
 		entry->private_data = chip;
 		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
@@ -871,7 +883,8 @@ int cs46xx_dsp_proc_init (struct snd_card *card, struct snd_cs46xx *chip)
 	}
 	ins->proc_task_info_entry = entry;
 
-	if ((entry = snd_info_create_card_entry(card, "scb_info", ins->proc_dsp_dir)) != NULL) {
+	entry = snd_info_create_card_entry(card, "scb_info", ins->proc_dsp_dir);
+	if (entry) {
 		entry->content = SNDRV_INFO_CONTENT_TEXT;
 		entry->private_data = chip;
 		entry->mode = S_IFREG | S_IRUGO | S_IWUSR;
diff --git a/sound/pci/cs46xx/dsp_spos_scb_lib.c b/sound/pci/cs46xx/dsp_spos_scb_lib.c
index 7488e1b7a770..f9564f42eb09 100644
--- a/sound/pci/cs46xx/dsp_spos_scb_lib.c
+++ b/sound/pci/cs46xx/dsp_spos_scb_lib.c
@@ -256,9 +256,10 @@ void cs46xx_dsp_proc_register_scb_desc (struct snd_cs46xx *chip,
 	/* register to proc */
 	if (ins->snd_card != NULL && ins->proc_dsp_dir != NULL &&
 	    scb->proc_info == NULL) {
-  
-		if ((entry = snd_info_create_card_entry(ins->snd_card, scb->scb_name, 
-							ins->proc_dsp_dir)) != NULL) {
+		entry = snd_info_create_card_entry(ins->snd_card,
+						   scb->scb_name,
+						   ins->proc_dsp_dir);
+		if (entry) {
 			scb_info = kmalloc(sizeof(struct proc_scb_info), GFP_KERNEL);
 			if (!scb_info) {
 				snd_info_free_entry(entry);
-- 
2.15.0

[PATCH 2/4] ALSA: cs46xx: Use common error handling code in two functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 13 Nov 2017 18:53:11 +0100

Add jump targets so that a bit of exception handling can be better reused
at the end of these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 sound/pci/cs46xx/cs46xx.c     | 64 ++++++++++++++++++-------------------------
 sound/pci/cs46xx/cs46xx_lib.c | 44 ++++++++++++++---------------
 2 files changed, 47 insertions(+), 61 deletions(-)

diff --git a/sound/pci/cs46xx/cs46xx.c b/sound/pci/cs46xx/cs46xx.c
index 520478a6e605..d42e7e282cab 100644
--- a/sound/pci/cs46xx/cs46xx.c
+++ b/sound/pci/cs46xx/cs46xx.c
@@ -95,54 +95,42 @@ static int snd_card_cs46xx_probe(struct pci_dev *pci,
 
 	err = snd_cs46xx_create(card, pci, external_amp[dev],
 				thinkpad[dev], &chip);
-	if (err < 0) {
-		snd_card_free(card);
-		return err;
-	}
+	if (err < 0)
+		goto free_card;
+
 	card->private_data = chip;
 	chip->accept_valid = mmap_valid[dev];
 	err = snd_cs46xx_pcm(chip, 0);
-	if (err < 0) {
-		snd_card_free(card);
-		return err;
-	}
+	if (err < 0)
+		goto free_card;
+
 #ifdef CONFIG_SND_CS46XX_NEW_DSP
 	err = snd_cs46xx_pcm_rear(chip, 1);
-	if (err < 0) {
-		snd_card_free(card);
-		return err;
-	}
+	if (err < 0)
+		goto free_card;
+
 	err = snd_cs46xx_pcm_iec958(chip, 2);
-	if (err < 0) {
-		snd_card_free(card);
-		return err;
-	}
+	if (err < 0)
+		goto free_card;
 #endif
 	err = snd_cs46xx_mixer(chip, 2);
-	if (err < 0) {
-		snd_card_free(card);
-		return err;
-	}
+	if (err < 0)
+		goto free_card;
+
 #ifdef CONFIG_SND_CS46XX_NEW_DSP
 	if (chip->nr_ac97_codecs ==2) {
 		err = snd_cs46xx_pcm_center_lfe(chip, 3);
-		if (err < 0) {
-			snd_card_free(card);
-			return err;
-		}
+		if (err < 0)
+			goto free_card;
 	}
 #endif
 	err = snd_cs46xx_midi(chip, 0);
-	if (err < 0) {
-		snd_card_free(card);
-		return err;
-	}
-	err = snd_cs46xx_start_dsp(chip);
-	if (err < 0) {
-		snd_card_free(card);
-		return err;
-	}
+	if (err < 0)
+		goto free_card;
 
+	err = snd_cs46xx_start_dsp(chip);
+	if (err < 0)
+		goto free_card;
 
 	snd_cs46xx_gameport(chip);
 
@@ -155,14 +143,16 @@ static int snd_card_cs46xx_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_card_cs46xx_remove(struct pci_dev *pci)
diff --git a/sound/pci/cs46xx/cs46xx_lib.c b/sound/pci/cs46xx/cs46xx_lib.c
index 164f86949b2d..27b568f350f6 100644
--- a/sound/pci/cs46xx/cs46xx_lib.c
+++ b/sound/pci/cs46xx/cs46xx_lib.c
@@ -3937,8 +3937,7 @@ int snd_cs46xx_create(struct snd_card *card,
 		dev_err(chip->card->dev,
 			"wrong address(es) - ba0 = 0x%lx, ba1 = 0x%lx\n",
 			   chip->ba0_addr, chip->ba1_addr);
-	    	snd_cs46xx_free(chip);
-	    	return -ENOMEM;
+		goto e_nomem;
 	}
 
 	region = &chip->region.name.ba0;
@@ -4016,59 +4015,56 @@ int snd_cs46xx_create(struct snd_card *card,
 			dev_err(chip->card->dev,
 				"unable to request memory region 0x%lx-0x%lx\n",
 				   region->base, region->base + region->size - 1);
-			snd_cs46xx_free(chip);
-			return -EBUSY;
+			err = -EBUSY;
+			goto free_sound_chip;
 		}
 		region->remap_addr = ioremap_nocache(region->base, region->size);
 		if (region->remap_addr == NULL) {
 			dev_err(chip->card->dev,
 				"%s ioremap problem\n", region->name);
-			snd_cs46xx_free(chip);
-			return -ENOMEM;
+			goto e_nomem;
 		}
 	}
 
 	if (request_irq(pci->irq, snd_cs46xx_interrupt, IRQF_SHARED,
 			KBUILD_MODNAME, chip)) {
 		dev_err(chip->card->dev, "unable to grab IRQ %d\n", pci->irq);
-		snd_cs46xx_free(chip);
-		return -EBUSY;
+		err = -EBUSY;
+		goto free_sound_chip;
 	}
 	chip->irq = pci->irq;
 
 #ifdef CONFIG_SND_CS46XX_NEW_DSP
 	chip->dsp_spos_instance = cs46xx_dsp_spos_create(chip);
-	if (chip->dsp_spos_instance == NULL) {
-		snd_cs46xx_free(chip);
-		return -ENOMEM;
-	}
+	if (!chip->dsp_spos_instance)
+		goto e_nomem;
 #endif
 
 	err = snd_cs46xx_chip_init(chip);
-	if (err < 0) {
-		snd_cs46xx_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_cs46xx_free(chip);
-		return err;
-	}
+	if (err < 0)
+		goto free_sound_chip;
 	
 	snd_cs46xx_proc_init(card, chip);
 
 #ifdef CONFIG_PM_SLEEP
 	chip->saved_regs = kmalloc(sizeof(*chip->saved_regs) *
 				   ARRAY_SIZE(saved_regs), GFP_KERNEL);
-	if (!chip->saved_regs) {
-		snd_cs46xx_free(chip);
-		return -ENOMEM;
-	}
+	if (!chip->saved_regs)
+		goto e_nomem;
 #endif
 
 	chip->active_ctrl(chip, -1); /* disable CLKRUN */
 
 	*rchip = chip;
 	return 0;
+
+e_nomem:
+	err = -ENOMEM;
+free_sound_chip:
+	snd_cs46xx_free(chip);
+	return err;
 }
-- 
2.15.0

[PATCH 3/4] ALSA: cs46xx: Improve a size determination in cs46xx_dsp_proc_register_scb_desc()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 13 Nov 2017 18:56:58 +0100

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 sound/pci/cs46xx/dsp_spos_scb_lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/pci/cs46xx/dsp_spos_scb_lib.c b/sound/pci/cs46xx/dsp_spos_scb_lib.c
index f9564f42eb09..bccd315e6c18 100644
--- a/sound/pci/cs46xx/dsp_spos_scb_lib.c
+++ b/sound/pci/cs46xx/dsp_spos_scb_lib.c
@@ -260,7 +260,7 @@ void cs46xx_dsp_proc_register_scb_desc (struct snd_cs46xx *chip,
 						   scb->scb_name,
 						   ins->proc_dsp_dir);
 		if (entry) {
-			scb_info = kmalloc(sizeof(struct proc_scb_info), GFP_KERNEL);
+			scb_info = kmalloc(sizeof(*scb_info), GFP_KERNEL);
 			if (!scb_info) {
 				snd_info_free_entry(entry);
 				entry = NULL;
-- 
2.15.0

[PATCH 4/4] ALSA: cs46xx: Adjust 35 checks for null pointers

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 13 Nov 2017 19:03:29 +0100
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written …

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 sound/pci/cs46xx/cs46xx_lib.c       | 21 +++++++++++----------
 sound/pci/cs46xx/dsp_spos.c         | 31 +++++++++++++++----------------
 sound/pci/cs46xx/dsp_spos_scb_lib.c | 27 +++++++++++----------------
 3 files changed, 37 insertions(+), 42 deletions(-)

diff --git a/sound/pci/cs46xx/cs46xx_lib.c b/sound/pci/cs46xx/cs46xx_lib.c
index 27b568f350f6..ddbd33dca4bc 100644
--- a/sound/pci/cs46xx/cs46xx_lib.c
+++ b/sound/pci/cs46xx/cs46xx_lib.c
@@ -1057,10 +1057,10 @@ static int _cs46xx_adjust_sample_rate (struct snd_cs46xx *chip, struct snd_cs46x
 {
 
 	/* If PCMReaderSCB and SrcTaskSCB not created yet ... */
-	if ( cpcm->pcm_channel == NULL) {
+	if (!cpcm->pcm_channel) {
 		cpcm->pcm_channel = cs46xx_dsp_create_pcm_channel (chip, sample_rate, 
 								   cpcm, cpcm->hw_buf.addr,cpcm->pcm_channel_id);
-		if (cpcm->pcm_channel == NULL) {
+		if (!cpcm->pcm_channel) {
 			dev_err(chip->card->dev,
 				"failed to create virtual PCM channel\n");
 			return -ENOMEM;
@@ -1514,7 +1514,7 @@ static int _cs46xx_playback_open_channel (struct snd_pcm_substream *substream,in
 	struct snd_pcm_runtime *runtime = substream->runtime;
 
 	cpcm = kzalloc(sizeof(*cpcm), GFP_KERNEL);
-	if (cpcm == NULL)
+	if (!cpcm)
 		return -ENOMEM;
 	if (snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, snd_dma_pci_data(chip->pci),
 				PAGE_SIZE, &cpcm->hw_buf) < 0) {
@@ -2063,7 +2063,7 @@ static int snd_cs46xx_adc_capture_get(struct snd_kcontrol *kcontrol,
 	struct snd_cs46xx *chip = snd_kcontrol_chip(kcontrol);
 	struct dsp_spos_instance * ins = chip->dsp_spos_instance;
 
-	if (ins->adc_input != NULL) 
+	if (ins->adc_input)
 		ucontrol->value.integer.value[0] = 1;
 	else 
 		ucontrol->value.integer.value[0] = 0;
@@ -2094,7 +2094,7 @@ static int snd_cs46xx_pcm_capture_get(struct snd_kcontrol *kcontrol,
 	struct snd_cs46xx *chip = snd_kcontrol_chip(kcontrol);
 	struct dsp_spos_instance * ins = chip->dsp_spos_instance;
 
-	if (ins->pcm_input != NULL) 
+	if (ins->pcm_input)
 		ucontrol->value.integer.value[0] = 1;
 	else 
 		ucontrol->value.integer.value[0] = 0;
@@ -3643,7 +3643,7 @@ static void clkrun_init(struct snd_cs46xx *chip)
 	
 	pdev = pci_get_device(PCI_VENDOR_ID_INTEL,
 		PCI_DEVICE_ID_INTEL_82371AB_3, NULL);
-	if (pdev == NULL)
+	if (!pdev)
 		return;		/* Not a thinkpad thats for sure */
 
 	/* Find the control port */		
@@ -3919,7 +3919,7 @@ int snd_cs46xx_create(struct snd_card *card,
 		return err;
 
 	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
-	if (chip == NULL) {
+	if (!chip) {
 		pci_disable_device(pci);
 		return -ENOMEM;
 	}
@@ -3997,9 +3997,10 @@ int snd_cs46xx_create(struct snd_card *card,
 		clkrun_init(chip);
 	}
 	
-	if (chip->amplifier_ctrl == NULL)
+	if (!chip->amplifier_ctrl)
 		chip->amplifier_ctrl = amp_none;
-	if (chip->active_ctrl == NULL)
+
+	if (!chip->active_ctrl)
 		chip->active_ctrl = amp_none;
 
 	chip->active_ctrl(chip, 1); /* enable CLKRUN */
@@ -4019,7 +4020,7 @@ int snd_cs46xx_create(struct snd_card *card,
 			goto free_sound_chip;
 		}
 		region->remap_addr = ioremap_nocache(region->base, region->size);
-		if (region->remap_addr == NULL) {
+		if (!region->remap_addr) {
 			dev_err(chip->card->dev,
 				"%s ioremap problem\n", region->name);
 			goto e_nomem;
diff --git a/sound/pci/cs46xx/dsp_spos.c b/sound/pci/cs46xx/dsp_spos.c
index c30bb557468c..63a212f3bc58 100644
--- a/sound/pci/cs46xx/dsp_spos.c
+++ b/sound/pci/cs46xx/dsp_spos.c
@@ -236,7 +236,7 @@ struct dsp_spos_instance *cs46xx_dsp_spos_create (struct snd_cs46xx * chip)
 {
 	struct dsp_spos_instance * ins = kzalloc(sizeof(struct dsp_spos_instance), GFP_KERNEL);
 
-	if (ins == NULL)
+	if (!ins)
 		return NULL;
 
 	/* better to use vmalloc for this big table */
@@ -402,7 +402,7 @@ int cs46xx_dsp_load_module (struct snd_cs46xx * chip, struct dsp_module_desc * m
 		snd_cs46xx_clear_BA1(chip, DSP_CODE_BYTE_OFFSET, DSP_CODE_BYTE_SIZE);
 	}
 
-	if (code == NULL) {
+	if (!code) {
 		dev_dbg(chip->card->dev,
 			"dsp_spos: module got no code segment\n");
 	} else {
@@ -514,10 +514,8 @@ static void cs46xx_dsp_proc_symbol_table_read (struct snd_info_entry *entry,
 		if (ins->symbol_table.symbols[i].deleted)
 			continue;
 
-		if (ins->symbol_table.symbols[i].module != NULL) {
+		if (ins->symbol_table.symbols[i].module)
 			module_str = ins->symbol_table.symbols[i].module->module_name;
-		}
-
     
 		snd_iprintf(buffer, "%04X <%02X> %s [%s]\n",
 			    ins->symbol_table.symbols[i].address,
@@ -593,11 +591,12 @@ static void cs46xx_dsp_proc_scb_read (struct snd_info_entry *entry,
 			continue;
 		snd_iprintf(buffer,"\n%04x %s:\n\n",ins->scbs[i].address,ins->scbs[i].scb_name);
 
-		if (ins->scbs[i].parent_scb_ptr != NULL) {
+		if (ins->scbs[i].parent_scb_ptr)
 			snd_iprintf(buffer,"parent [%s:%04x] ", 
 				    ins->scbs[i].parent_scb_ptr->scb_name,
 				    ins->scbs[i].parent_scb_ptr->address);
-		} else snd_iprintf(buffer,"parent [none] ");
+		else
+			snd_iprintf(buffer, "parent [none] ");
 
 		snd_iprintf(buffer,"sub_list_ptr [%s:%04x]\nnext_scb_ptr [%s:%04x]  task_entry [%s:%04x]\n",
 			    ins->scbs[i].sub_list_ptr->scb_name,
@@ -1141,35 +1140,35 @@ int cs46xx_dsp_scb_and_task_init (struct snd_cs46xx *chip)
 	cs46xx_dsp_create_task_tree(chip, "sposCB", (u32 *)&sposcb, SPOSCB_ADDR, 0x10);
 
 	null_algorithm  = cs46xx_dsp_lookup_symbol(chip, "NULLALGORITHM", SYMBOL_CODE);
-	if (null_algorithm == NULL) {
+	if (!null_algorithm) {
 		dev_err(chip->card->dev,
 			"dsp_spos: symbol NULLALGORITHM not found\n");
 		return -EIO;
 	}
 
 	fg_task_tree_header_code = cs46xx_dsp_lookup_symbol(chip, "FGTASKTREEHEADERCODE", SYMBOL_CODE);  
-	if (fg_task_tree_header_code == NULL) {
+	if (!fg_task_tree_header_code) {
 		dev_err(chip->card->dev,
 			"dsp_spos: symbol FGTASKTREEHEADERCODE not found\n");
 		return -EIO;
 	}
 
 	task_tree_header_code = cs46xx_dsp_lookup_symbol(chip, "TASKTREEHEADERCODE", SYMBOL_CODE);  
-	if (task_tree_header_code == NULL) {
+	if (!task_tree_header_code) {
 		dev_err(chip->card->dev,
 			"dsp_spos: symbol TASKTREEHEADERCODE not found\n");
 		return -EIO;
 	}
   
 	task_tree_thread = cs46xx_dsp_lookup_symbol(chip, "TASKTREETHREAD", SYMBOL_CODE);
-	if (task_tree_thread == NULL) {
+	if (!task_tree_thread) {
 		dev_err(chip->card->dev,
 			"dsp_spos: symbol TASKTREETHREAD not found\n");
 		return -EIO;
 	}
 
 	magic_snoop_task = cs46xx_dsp_lookup_symbol(chip, "MAGICSNOOPTASK", SYMBOL_CODE);
-	if (magic_snoop_task == NULL) {
+	if (!magic_snoop_task) {
 		dev_err(chip->card->dev,
 			"dsp_spos: symbol MAGICSNOOPTASK not found\n");
 		return -EIO;
@@ -1532,20 +1531,20 @@ static int cs46xx_dsp_async_init (struct snd_cs46xx *chip,
 	struct dsp_scb_descriptor * spdifi_scb_desc, * spdifo_scb_desc, * async_codec_scb_desc;
 
 	s16_async_codec_input_task = cs46xx_dsp_lookup_symbol(chip, "S16_ASYNCCODECINPUTTASK", SYMBOL_CODE);
-	if (s16_async_codec_input_task == NULL) {
+	if (!s16_async_codec_input_task) {
 		dev_err(chip->card->dev,
 			"dsp_spos: symbol S16_ASYNCCODECINPUTTASK not found\n");
 		return -EIO;
 	}
 	spdifo_task = cs46xx_dsp_lookup_symbol(chip, "SPDIFOTASK", SYMBOL_CODE);
-	if (spdifo_task == NULL) {
+	if (!spdifo_task) {
 		dev_err(chip->card->dev,
 			"dsp_spos: symbol SPDIFOTASK not found\n");
 		return -EIO;
 	}
 
 	spdifi_task = cs46xx_dsp_lookup_symbol(chip, "SPDIFITASK", SYMBOL_CODE);
-	if (spdifi_task == NULL) {
+	if (!spdifi_task) {
 		dev_err(chip->card->dev,
 			"dsp_spos: symbol SPDIFITASK not found\n");
 		return -EIO;
@@ -1971,7 +1970,7 @@ int cs46xx_dsp_set_iec958_volume (struct snd_cs46xx * chip, u16 left, u16 right)
 
 	mutex_lock(&chip->spos_mutex);
 
-	if (ins->asynch_rx_scb != NULL)
+	if (ins->asynch_rx_scb)
 		cs46xx_dsp_scb_set_volume (chip,ins->asynch_rx_scb,
 					   left,right);
 
diff --git a/sound/pci/cs46xx/dsp_spos_scb_lib.c b/sound/pci/cs46xx/dsp_spos_scb_lib.c
index bccd315e6c18..e42f402fbe4d 100644
--- a/sound/pci/cs46xx/dsp_spos_scb_lib.c
+++ b/sound/pci/cs46xx/dsp_spos_scb_lib.c
@@ -93,7 +93,7 @@ static void cs46xx_dsp_proc_scb_info_read (struct snd_info_entry *entry,
   
 	snd_iprintf(buffer,"\n");
 
-	if (scb->parent_scb_ptr != NULL) {
+	if (scb->parent_scb_ptr) {
 		snd_iprintf(buffer,"parent [%s:%04x] ", 
 			    scb->parent_scb_ptr->scb_name,
 			    scb->parent_scb_ptr->address);
@@ -254,8 +254,7 @@ void cs46xx_dsp_proc_register_scb_desc (struct snd_cs46xx *chip,
 	struct proc_scb_info * scb_info;
 
 	/* register to proc */
-	if (ins->snd_card != NULL && ins->proc_dsp_dir != NULL &&
-	    scb->proc_info == NULL) {
+	if (ins->snd_card && ins->proc_dsp_dir && !scb->proc_info) {
 		entry = snd_info_create_card_entry(ins->snd_card,
 						   scb->scb_name,
 						   ins->proc_dsp_dir);
@@ -376,8 +375,7 @@ cs46xx_dsp_create_generic_scb (struct snd_cs46xx *chip, char * name, u32 * scb_d
 
 	task_entry = cs46xx_dsp_lookup_symbol (chip,task_entry_name,
 					       SYMBOL_CODE);
-  
-	if (task_entry == NULL) {
+	if (!task_entry) {
 		dev_err(chip->card->dev,
 			"dsp_spos: symbol %s not found\n", task_entry_name);
 		return NULL;
@@ -588,11 +586,10 @@ cs46xx_dsp_create_pcm_reader_scb(struct snd_cs46xx * chip, char * scb_name,
 		}
 	};
 
-	if (ins->null_algorithm == NULL) {
+	if (!ins->null_algorithm) {
 		ins->null_algorithm =  cs46xx_dsp_lookup_symbol (chip,"NULLALGORITHM",
 								 SYMBOL_CODE);
-    
-		if (ins->null_algorithm == NULL) {
+		if (!ins->null_algorithm) {
 			dev_err(chip->card->dev,
 				"dsp_spos: symbol NULLALGORITHM not found\n");
 			return NULL;
@@ -678,11 +675,10 @@ cs46xx_dsp_create_src_task_scb(struct snd_cs46xx * chip, char * scb_name,
 			}
 		};
 		
-		if (ins->s16_up == NULL) {
+		if (!ins->s16_up) {
 			ins->s16_up =  cs46xx_dsp_lookup_symbol (chip,"S16_UPSRC",
 								 SYMBOL_CODE);
-			
-			if (ins->s16_up == NULL) {
+			if (!ins->s16_up) {
 				dev_err(chip->card->dev,
 					"dsp_spos: symbol S16_UPSRC not found\n");
 				return NULL;
@@ -1292,9 +1288,8 @@ cs46xx_dsp_create_pcm_channel (struct snd_cs46xx * chip,
 	if (!sample_rate) sample_rate = 44100;
 
 	/* search for a already created SRC SCB with the same sample rate */
-	for (i = 0; i < DSP_MAX_PCM_CHANNELS && 
-		     (pcm_index == -1 || src_scb == NULL); ++i) {
-
+	for (i = 0; i < DSP_MAX_PCM_CHANNELS && (pcm_index == -1 || !src_scb);
+	     ++i) {
 		/* virtual channel reserved 
 		   for capture */
 		if (i == CS46XX_DSP_CAPTURE_CHANNEL) continue;
@@ -1317,7 +1312,7 @@ cs46xx_dsp_create_pcm_channel (struct snd_cs46xx * chip,
 		return NULL;
 	}
 
-	if (src_scb == NULL) {
+	if (!src_scb) {
 		if (ins->nsrc_scb >= DSP_MAX_SRC_NR) {
 			dev_err(chip->card->dev,
 				"dsp_spos: to many SRC instances\n!");
@@ -1784,7 +1779,7 @@ int cs46xx_iec958_post_close (struct snd_cs46xx *chip)
 	cs46xx_poke_via_dsp (chip,SP_SPDOUT_CSUV, ins->spdif_csuv_default);
 	
 	/* deallocate stuff */
-	if (ins->spdif_pcm_input_scb != NULL) {
+	if (ins->spdif_pcm_input_scb) {
 		cs46xx_dsp_remove_scb (chip,ins->spdif_pcm_input_scb);
 		ins->spdif_pcm_input_scb = NULL;
 	}
-- 
2.15.0

Re: [PATCH 1/4] ALSA: cs46xx: Adjust 33 function calls together with a variable assignment

[Dan Carpenter]

On Mon, Nov 13, 2017 at 07:22:30PM +0100, SF Markus Elfring wrote:
> -
> -		if ( (cpcm->pcm_channel = cs46xx_dsp_create_pcm_channel (chip, sample_rate, cpcm, 
> -									 cpcm->hw_buf.addr,
> -									 cpcm->pcm_channel_id)) == NULL) {
> +		cpcm
> +		->pcm_channel = cs46xx_dsp_create_pcm_channel(chip,
                ^^^^^^^^^^^^^
Please don't break it up like this

> +							      sample_rate,
> +							      cpcm,
> +							      cpcm->hw_buf.addr,
> +							      cpcm
> +							      ->pcm_channel_id);
                                                              ^^^^^^^^^^^^^^^^
or this.  It's better to go over 80 characters than to do that.

regards,
dan carpenter