[alsa-devel] [PATCH 1/5] ALSA: pcm: Fix poll error return codes

[Charles Keepax]

On Thu, May 05, 2016 at 11:39:34AM +0200, Clemens Ladisch wrote:
> Takashi Sakamoto wrote:
> > On May 4 2016 22:59, Charles Keepax wrote:
> >>  	if (PCM_RUNTIME_CHECK(substream))
> >> -		return -ENXIO;
> >> +		return POLLIN | POLLRDNORM | POLLERR;
> >
> > [...]
> > On the other hand, I think POLLOUT, POLLIN, POLLWRNORM and POLLRDNORM
> > should not be included in the value. PCM_RUNTIME_CHECK() ensures PCM
> > substream or PCM runtime is NULL. This means that subsequent I/O
> > operations are failed, at least for handling PCM frames.
> >
> > I think it better to return 'POLLERR | POLLHUP'.
> 
> To expand on this: POLLIN/POLLOUT imply that it is possible to read/
> write data without blocking.  Sockets and pipes combine POLLHUP with
> POLLIN because the read() (or recv()) returns 0 bytes without blocking
> to indicate the end of the stream.
> 
> But in this situation, snd_pcm_read*/write* will always fail, so it is,
> strictly speaking, indeed not appropriate to set POLLIN/OUT.
> 
> On the other hand, PCM devices do include the POLLIN/OUT bits when they
> are in a wrong state.  (This is probably to catch programs that do not
> check the error bits; with POLLIN/OUT set, these programs will try to
> read/write, and will then get the error code.)
> 
> So for consistency, the bits should be included.  (Or the other error
> case fixed to remove these bits.)

Thanks for the explaination guys. I definitely agree that all
the return values should be consistent. I am happy to change the
values if people prefer but I guess the decision really rests
with Takashi and if he is happy to change the returns to
POLLERR | POLLHUP, as I guess there is the potential for some
user-space fall out. Perhaps I should do this as a seperate patch
on top of this chain, so we can review explicitly.

I have had a look and both tinyalsa and alsalib look like they
would handle the change correctly.

Thanks,
Charles