2023-04-17 05:48:41

by Christophe JAILLET

Subject: [PATCH] ALSA: hda/hdmi: Remove some dead code

These snd_BUG_ON() can never trigger, so just remove them to save a few
LoC.

Signed-off-by: Christophe JAILLET <[email protected]>
---
This patch is speculative. Maybe the other "if"s should be removed and the
error code returned instead.

I chose to remove these ones and the corresponding error code, because it
seems that it is already handled this way in hdmi_pcm_open().

I don't know the code enough, so it is only my guess.

Review with care :)
---
sound/pci/hda/patch_hdmi.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 5c6980394dce..ee051bdfaff6 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -2104,10 +2104,6 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
goto unlock;
}

- if (snd_BUG_ON(pin_idx < 0)) {
- err = -EINVAL;
- goto unlock;
- }
per_pin = get_pin(spec, pin_idx);

/* Verify pin:cvt selections to avoid silent audio after S3.
@@ -2199,13 +2195,13 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
snd_hda_spdif_ctls_unassign(codec, pcm_idx);
clear_bit(pcm_idx, &spec->pcm_in_use);
pin_idx = hinfo_to_pin_index(codec, hinfo);
+ /*
+ * In such a case, return 0 to match the behavior in
+ * hdmi_pcm_open()
+ */
if (pin_idx < 0)
goto unlock;

- if (snd_BUG_ON(pin_idx < 0)) {
- err = -EINVAL;
- goto unlock;
- }
per_pin = get_pin(spec, pin_idx);

if (spec->dyn_pin_out) {
--
2.34.1

2023-04-18 06:01:55

by Takashi Iwai

Subject: Re: [PATCH] ALSA: hda/hdmi: Remove some dead code

On Mon, 17 Apr 2023 07:41:48 +0200,
Christophe JAILLET wrote:
>
> These snd_BUG_ON() can never trigger, so just remove them to save a few
> LoC.
>
> Signed-off-by: Christophe JAILLET <[email protected]>
> ---
> This patch is speculative. Maybe the other "if"s should be removed and the
> error code returned instead.
>
> I chose to remove these ones and the corresponding error code, because it
> seems that it is already handled this way in hdmi_pcm_open().
>
> I don't know the code enough, so it is only my guess.
>
> Review with care :)

A sanity check is merely a sanity check, and we haven't got report
hitting this, so this should be fine. Let's cross fingers.


thanks,

Takashi