Re: [PATCH 1/5] ALSA: pcm: Fix poll error return codes

[Takashi Sakamoto <o-takashi@sakamocchi.jp>]

Hi,

On May 4 2016 22:59, Charles Keepax wrote:
> We can't return a negative error code from the poll callback the return
> type is unsigned and is checked against the poll specific flags we need
> to return POLLERR if we encounter an error.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  sound/core/pcm_native.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index 9106d8e..c61fd50 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -3161,7 +3161,7 @@ static unsigned int snd_pcm_playback_poll(struct file *file, poll_table * wait)
>  
>  	substream = pcm_file->substream;
>  	if (PCM_RUNTIME_CHECK(substream))
> -		return -ENXIO;
> +		return POLLOUT | POLLWRNORM | POLLERR;
>  	runtime = substream->runtime;
>  
>  	poll_wait(file, &runtime->sleep, wait);
> @@ -3200,7 +3200,7 @@ static unsigned int snd_pcm_capture_poll(struct file *file, poll_table * wait)
>  
>  	substream = pcm_file->substream;
>  	if (PCM_RUNTIME_CHECK(substream))
> -		return -ENXIO;
> +		return POLLIN | POLLRDNORM | POLLERR;
>  	runtime = substream->runtime;
>  
>  	poll_wait(file, &runtime->sleep, wait);

I agree with the concept of your patch to fix the return value of ALSA
PCM core. It should return a value which consists of POLLxxx masks.

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'.


Regards

Takashi Sakamoto