From: Cezary Rojewski <cezary.rojewski@intel.com>
To: <phucduc.bui@gmail.com>
Cc: <cassiogabrielcontato@gmail.com>, Mark Brown <broonie@kernel.org>,
"Kees Cook" <kees@kernel.org>, <linux-sound@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, Jaroslav Kysela <perex@perex.cz>,
"Takashi Iwai" <tiwai@suse.com>
Subject: Re: [PATCH v2] ALSA: control: preserve snd_card_file_add() error code in snd_ctl_open()
Date: Wed, 8 Jul 2026 13:02:36 +0200 [thread overview]
Message-ID: <f6841e94-3260-4ec0-bdef-50ed73ba8875@intel.com> (raw)
In-Reply-To: <20260708023540.6962-1-phucduc.bui@gmail.com>
On 7/8/2026 4:35 AM, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
>
> snd_ctl_open() unconditionally overwrites the return value of
> snd_card_file_add() with -ENODEV on failure, discarding the actual
> error code.
>
> Fix this by directly returning the original error code returned by
> snd_card_file_add() (e.g. -ENOMEM or -ENODEV). This behavior is
> consistent with the error handling used in other functions such as
> snd_mixer_oss_open(), snd_hwdep_open(), snd_pcm_oss_open(), and others.
>
> There is no functional change other than the returned error code in this
> failure path.
BTW, as you've provided a number of positive contributions, a tip for
future:
The change is simple and does not relate to any workaround or some kind
of 'recommended' sequence. And thus there's no need to be as verbose.
IMHO the first paragraph suffices - explicitly answers the _why_ question.
The last is completely redundant - return code is a functional change.
Similarly, "Fix this by (...)" in context of such a simple change yields
no value for the reader - we see _what_ is happening clearly in the code.
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---
>
> Changes in v2:
> - Update commit messages.
> - Drop superfluous braces.
>
> sound/core/control.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/sound/core/control.c b/sound/core/control.c
> index 0ca9fff56e51..73d7ba0f509f 100644
> --- a/sound/core/control.c
> +++ b/sound/core/control.c
> @@ -63,10 +63,8 @@ static int snd_ctl_open(struct inode *inode, struct file *file)
> goto __error1;
> }
> err = snd_card_file_add(card, file);
> - if (err < 0) {
> - err = -ENODEV;
> + if (err < 0)
> goto __error1;
> - }
> if (!try_module_get(card->module)) {
> err = -ENODEV;
> goto __error2;
next prev parent reply other threads:[~2026-07-08 11:02 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-08 2:35 [PATCH v2] ALSA: control: preserve snd_card_file_add() error code in snd_ctl_open() phucduc.bui
2026-07-08 6:53 ` Cezary Rojewski
2026-07-08 10:49 ` Takashi Iwai
2026-07-08 11:02 ` Cezary Rojewski [this message]
2026-07-08 13:10 ` Bui Duc Phuc
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f6841e94-3260-4ec0-bdef-50ed73ba8875@intel.com \
--to=cezary.rojewski@intel.com \
--cc=broonie@kernel.org \
--cc=cassiogabrielcontato@gmail.com \
--cc=kees@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sound@vger.kernel.org \
--cc=perex@perex.cz \
--cc=phucduc.bui@gmail.com \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox