[PATCH] ALSA: oss: Drop redundant nonseekable_open() return check

[PATCH] ALSA: oss: Drop redundant nonseekable_open() return check

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

nonseekable_open() always returns 0, so the error check is unnecessary.
Remove the dead error handling path.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/core/oss/mixer_oss.c | 2 --
 sound/core/oss/pcm_oss.c   | 2 --
 2 files changed, 4 deletions(-)

diff --git a/sound/core/oss/mixer_oss.c b/sound/core/oss/mixer_oss.c
index 591cff800329..6161740f67fa 100644
--- a/sound/core/oss/mixer_oss.c
+++ b/sound/core/oss/mixer_oss.c
@@ -31,8 +31,6 @@ static int snd_mixer_oss_open(struct inode *inode, struct file *file)
 	int err;
 
 	err = nonseekable_open(inode, file);
-	if (err < 0)
-		return err;
 
 	card = snd_lookup_oss_minor_data(iminor(inode),
 					 SNDRV_OSS_DEVICE_TYPE_MIXER);
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index 9826d9a0be6d..a0bbfa0a7ec8 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -2496,8 +2496,6 @@ static int snd_pcm_oss_open(struct inode *inode, struct file *file)
 	wait_queue_entry_t wait;
 
 	err = nonseekable_open(inode, file);
-	if (err < 0)
-		return err;
 
 	pcm = snd_lookup_oss_minor_data(iminor(inode),
 					SNDRV_OSS_DEVICE_TYPE_PCM);
-- 
2.43.0

Re: [PATCH] ALSA: oss: Drop redundant nonseekable_open() return check

[Dan Carpenter]

On Thu, Jul 02, 2026 at 12:02:36PM +0700, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> nonseekable_open() always returns 0, so the error check is unnecessary.
> Remove the dead error handling path.

The idea is that it might return errors in the future.

Also the patch is kind of sloppy...

regards,
dan carpenter

Re: [PATCH] ALSA: oss: Drop redundant nonseekable_open() return check

[Dan Carpenter]

On Thu, Jul 09, 2026 at 01:00:55PM +0300, Dan Carpenter wrote:
> On Thu, Jul 02, 2026 at 12:02:36PM +0700, phucduc.bui@gmail.com wrote:
> > From: bui duc phuc <phucduc.bui@gmail.com>
> > 
> > nonseekable_open() always returns 0, so the error check is unnecessary.
> > Remove the dead error handling path.
> 
> The idea is that it might return errors in the future.
> 

Actually the comments on nonseekable_open() say there is no need to
check.  That would have been useful information to add in the commit
message...

> Also the patch is kind of sloppy...

The sloppiness was fixed in v2.  It's weird that there were no review
comments on lore for this patch.  Are people sending private reviews
now?

regards,
dan carpenter

Re: [PATCH] ALSA: oss: Drop redundant nonseekable_open() return check

[Bui Duc Phuc]

Hi Dan Carpenter,

Thank you for your feedback.

> >
> > The idea is that it might return errors in the future.
> >

I looked at the history of nonseekable_open(). It has existed since Linux 2.6
days, for over 20 years, and its behavior has remained  unchanged.
The comment still explicitly states:
"The function is not supposed to ever fail..."
It also explains that the function returns int only so that it can be plugged
directly into file_operations, not because callers are expected to
handle errors.

I also checked the current kernel tree. nonseekable_open() is
referenced from 86 files,
and the overwhelming majority of callers ignore its return value
entirely, following the
documented API contract. Only three callers checked the return value:

+ GPIO (already applied):
https://lore.kernel.org/all/178332860255.18708.5813708689955494780.b4-ty@oss.qualcomm.com/
+ DRM:
https://lore.kernel.org/all/20260703020919.7958-1-phucduc.bui@gmail.com/
+ASoC:
https://lore.kernel.org/all/20260702084445.519669-1-phucduc.bui@gmail.com/

If nonseekable_open() were ever changed to return real errors, that
would change its
long-established API contract and would affect a large number of
existing callers.
Such a change would require updating those callers across the kernel,
rather than
relying on a few isolated defensive checks.

Given its documented semantics, its unchanged behavior for over two decades,
and how it is actually used throughout the kernel, I don't think
keeping redundant
error handling here provides any practical benefit.


>
> > Also the patch is kind of sloppy...
>
> The sloppiness was fixed in v2.  It's weird that there were no review
> comments on lore for this patch.  Are people sending private reviews
> now?

I was reading through the code and creating patches at the same time,
so I submitted cleanups as I found them. That made the earlier patches
somewhat inconsistent.
After Takashi-san reviewed one of the patches, I applied the same improvements
to the rest of the series and sent them in v2.
There were no private reviews. The feedback was entirely public:
https://lore.kernel.org/all/87echlam6x.wl-tiwai@suse.de/

Best regards,
Phuc