[PATCH 0/5] ALSA: Allow NULL passed to snd_ctl_remove()

[PATCH 0/5] ALSA: Allow NULL passed to snd_ctl_remove()

[Takashi Iwai]

This is a small series of cleanup patches for the call patterns with
snd_ctl_remove() and NULL kcontrol.
Now snd_ctl_remove() accepts the NULL argument just like free() or
kfree(), so that the callers can be cleaned up without NULL checks.

No functional changes should be introduced by this patch set.


Takashi

===

Takashi Iwai (5):
  ALSA: control: Allow NULL passed to snd_ctl_remove()
  ALSA: sb: Drop NULL check for snd_ctl_remove()
  ALSA: emu10k1:  Drop NULL check for snd_ctl_remove()
  ALSA: hda: Drop NULL check for snd_ctl_remove()
  ALSA: spi: Drop NULL check for snd_ctl_remove()

 sound/core/control.c            |  3 +++
 sound/isa/sb/emu8000.c          |  6 ++----
 sound/isa/sb/sb16_csp.c         | 12 ++++--------
 sound/pci/emu10k1/emufx.c       |  5 +----
 sound/pci/hda/hda_cs_dsp_ctl.c  |  2 +-
 sound/pci/hda/tas2781_hda_i2c.c | 13 ++++---------
 sound/spi/at73c213.c            |  8 ++------
 7 files changed, 17 insertions(+), 32 deletions(-)

-- 
2.43.0

[PATCH 1/5] ALSA: control: Allow NULL passed to snd_ctl_remove()

[Takashi Iwai]

There are lots of code checking NULL for kcontrol passed to
snd_ctl_remove() in the caller side.  Let's make snd_ctl_remove()
accepting the NULL kcontrol instead a la free(), so that we can clean
up the caller side.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/control.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/core/control.c b/sound/core/control.c
index fb0c60044f7b..7f0999d1671e 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -604,6 +604,7 @@ static inline int snd_ctl_remove_locked(struct snd_card *card,
  *
  * Removes the control from the card and then releases the instance.
  * You don't need to call snd_ctl_free_one().
+ * Passing NULL to @kcontrol argument is allowed as noop.
  *
  * Return: 0 if successful, or a negative error code on failure.
  *
@@ -611,6 +612,8 @@ static inline int snd_ctl_remove_locked(struct snd_card *card,
  */
 int snd_ctl_remove(struct snd_card *card, struct snd_kcontrol *kcontrol)
 {
+	if (!kcontrol)
+		return 0;
 	guard(rwsem_write)(&card->controls_rwsem);
 	return snd_ctl_remove_locked(card, kcontrol);
 }
-- 
2.43.0

[PATCH 2/5] ALSA: sb: Drop NULL check for snd_ctl_remove()

[Takashi Iwai]

Since snd_ctl_remove() accepts the NULL kcontrol argument now, we can
drop the check in the caller side.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/isa/sb/emu8000.c  |  6 ++----
 sound/isa/sb/sb16_csp.c | 12 ++++--------
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/sound/isa/sb/emu8000.c b/sound/isa/sb/emu8000.c
index a6405772d537..af478c36ce5b 100644
--- a/sound/isa/sb/emu8000.c
+++ b/sound/isa/sb/emu8000.c
@@ -1039,10 +1039,8 @@ snd_emu8000_create_mixer(struct snd_card *card, struct snd_emu8000 *emu)
 	return 0;
 
 __error:
-	for (i = 0; i < EMU8000_NUM_CONTROLS; i++) {
-		if (emu->controls[i])
-			snd_ctl_remove(card, emu->controls[i]);
-	}
+	for (i = 0; i < EMU8000_NUM_CONTROLS; i++)
+		snd_ctl_remove(card, emu->controls[i]);
 	return err;
 }
 
diff --git a/sound/isa/sb/sb16_csp.c b/sound/isa/sb/sb16_csp.c
index 8d8357019719..fdb992733bde 100644
--- a/sound/isa/sb/sb16_csp.c
+++ b/sound/isa/sb/sb16_csp.c
@@ -1080,14 +1080,10 @@ static void snd_sb_qsound_destroy(struct snd_sb_csp * p)
 
 	card = p->chip->card;	
 	
-	if (p->qsound_switch) {
-		snd_ctl_remove(card, p->qsound_switch);
-		p->qsound_switch = NULL;
-	}
-	if (p->qsound_space) {
-		snd_ctl_remove(card, p->qsound_space);
-		p->qsound_space = NULL;
-	}
+	snd_ctl_remove(card, p->qsound_switch);
+	p->qsound_switch = NULL;
+	snd_ctl_remove(card, p->qsound_space);
+	p->qsound_space = NULL;
 
 	/* cancel pending transfer of QSound parameters */
 	spin_lock_irqsave (&p->q_lock, flags);
-- 
2.43.0

[PATCH 3/5] ALSA: emu10k1: Drop NULL check for snd_ctl_remove()

[Takashi Iwai]

Since snd_ctl_remove() accepts the NULL kcontrol argument now, we can
drop the check in the caller side.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/emu10k1/emufx.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/sound/pci/emu10k1/emufx.c b/sound/pci/emu10k1/emufx.c
index 03efc317e05f..34f385b5cd46 100644
--- a/sound/pci/emu10k1/emufx.c
+++ b/sound/pci/emu10k1/emufx.c
@@ -959,7 +959,6 @@ static int snd_emu10k1_del_controls(struct snd_emu10k1 *emu,
 {
 	unsigned int i;
 	struct emu10k1_ctl_elem_id id;
-	struct snd_emu10k1_fx8010_ctl *ctl;
 	struct snd_card *card = emu->card;
 	int err;
 	
@@ -968,9 +967,7 @@ static int snd_emu10k1_del_controls(struct snd_emu10k1 *emu,
 				       in_kernel);
 		if (err < 0)
 			return err;
-		ctl = snd_emu10k1_look_for_ctl(emu, &id);
-		if (ctl)
-			snd_ctl_remove(card, ctl->kcontrol);
+		snd_ctl_remove(card, snd_emu10k1_look_for_ctl(emu, &id));
 	}
 	return 0;
 }
-- 
2.43.0

[PATCH 4/5] ALSA: hda: Drop NULL check for snd_ctl_remove()

[Takashi Iwai]

Since snd_ctl_remove() accepts the NULL kcontrol argument now, we can
drop the check in the caller side.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/hda_cs_dsp_ctl.c  |  2 +-
 sound/pci/hda/tas2781_hda_i2c.c | 13 ++++---------
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/sound/pci/hda/hda_cs_dsp_ctl.c b/sound/pci/hda/hda_cs_dsp_ctl.c
index e6e876998e71..deb74c247082 100644
--- a/sound/pci/hda/hda_cs_dsp_ctl.c
+++ b/sound/pci/hda/hda_cs_dsp_ctl.c
@@ -207,7 +207,7 @@ void hda_cs_dsp_control_remove(struct cs_dsp_coeff_ctl *cs_ctl)
 	struct hda_cs_dsp_coeff_ctl *ctl = cs_ctl->priv;
 
 	/* ctl and kctl may already have been removed by ALSA private_free */
-	if (ctl && ctl->kctl)
+	if (ctl)
 		snd_ctl_remove(ctl->card, ctl->kctl);
 }
 EXPORT_SYMBOL_NS_GPL(hda_cs_dsp_control_remove, SND_HDA_CS_DSP_CONTROLS);
diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c
index 75f7674c66ee..c878967cb66a 100644
--- a/sound/pci/hda/tas2781_hda_i2c.c
+++ b/sound/pci/hda/tas2781_hda_i2c.c
@@ -597,18 +597,13 @@ static void tas2781_hda_remove_controls(struct tas2781_hda *tas_hda)
 {
 	struct hda_codec *codec = tas_hda->priv->codec;
 
-	if (tas_hda->dsp_prog_ctl)
-		snd_ctl_remove(codec->card, tas_hda->dsp_prog_ctl);
-
-	if (tas_hda->dsp_conf_ctl)
-		snd_ctl_remove(codec->card, tas_hda->dsp_conf_ctl);
+	snd_ctl_remove(codec->card, tas_hda->dsp_prog_ctl);
+	snd_ctl_remove(codec->card, tas_hda->dsp_conf_ctl);
 
 	for (int i = ARRAY_SIZE(tas_hda->snd_ctls) - 1; i >= 0; i--)
-		if (tas_hda->snd_ctls[i])
-			snd_ctl_remove(codec->card, tas_hda->snd_ctls[i]);
+		snd_ctl_remove(codec->card, tas_hda->snd_ctls[i]);
 
-	if (tas_hda->prof_ctl)
-		snd_ctl_remove(codec->card, tas_hda->prof_ctl);
+	snd_ctl_remove(codec->card, tas_hda->prof_ctl);
 }
 
 static void tasdev_fw_ready(const struct firmware *fmw, void *context)
-- 
2.43.0

[PATCH 5/5] ALSA: spi: Drop NULL check for snd_ctl_remove()

[Takashi Iwai]

Since snd_ctl_remove() accepts the NULL kcontrol argument now, we can
drop the check in the caller side.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/spi/at73c213.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/sound/spi/at73c213.c b/sound/spi/at73c213.c
index 5648d744aa79..8f6929ced2c8 100644
--- a/sound/spi/at73c213.c
+++ b/sound/spi/at73c213.c
@@ -726,12 +726,8 @@ static int snd_at73c213_mixer(struct snd_at73c213 *chip)
 	return 0;
 
 cleanup:
-	for (idx = 1; idx < ARRAY_SIZE(snd_at73c213_controls) + 1; idx++) {
-		struct snd_kcontrol *kctl;
-		kctl = snd_ctl_find_numid(card, idx);
-		if (kctl)
-			snd_ctl_remove(card, kctl);
-	}
+	for (idx = 1; idx < ARRAY_SIZE(snd_at73c213_controls) + 1; idx++)
+		snd_ctl_remove(card, snd_ctl_find_numid(card, idx));
 	return errval;
 }
 
-- 
2.43.0

Re: [PATCH 3/5] ALSA: emu10k1: Drop NULL check for snd_ctl_remove()

[Takashi Iwai]

On Mon, 17 Jun 2024 12:05:25 +0200,
Takashi Iwai wrote:
> 
> Since snd_ctl_remove() accepts the NULL kcontrol argument now, we can
> drop the check in the caller side.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

Grrr, this is utterly broken.  Scratch this one.
Sorry for the noise.


Takashi

> ---
>  sound/pci/emu10k1/emufx.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/sound/pci/emu10k1/emufx.c b/sound/pci/emu10k1/emufx.c
> index 03efc317e05f..34f385b5cd46 100644
> --- a/sound/pci/emu10k1/emufx.c
> +++ b/sound/pci/emu10k1/emufx.c
> @@ -959,7 +959,6 @@ static int snd_emu10k1_del_controls(struct snd_emu10k1 *emu,
>  {
>  	unsigned int i;
>  	struct emu10k1_ctl_elem_id id;
> -	struct snd_emu10k1_fx8010_ctl *ctl;
>  	struct snd_card *card = emu->card;
>  	int err;
>  	
> @@ -968,9 +967,7 @@ static int snd_emu10k1_del_controls(struct snd_emu10k1 *emu,
>  				       in_kernel);
>  		if (err < 0)
>  			return err;
> -		ctl = snd_emu10k1_look_for_ctl(emu, &id);
> -		if (ctl)
> -			snd_ctl_remove(card, ctl->kcontrol);
> +		snd_ctl_remove(card, snd_emu10k1_look_for_ctl(emu, &id));
>  	}
>  	return 0;
>  }
> -- 
> 2.43.0
>