At Mon, 02 Feb 2009 15:34:57 +0100, I wrote:
At Mon, 02 Feb 2009 08:02:29 +0100, I wrote:
At Mon, 2 Feb 2009 03:50:22 +0100, Lennart Poettering wrote:
Heya!
If we look into the pcm example how snd_pcm_poll_descriptors_revents() is used then we can see that the revents parameter apparently is supposed to be a single integer. (which makes a lot of sense to me)
http://www.alsa-project.org/alsa-doc/alsa-lib/_2test_2pcm_8c-example.html#a3...
However, snd_pcm_wait_nocheck() calls the same function and assumes it is a complete array!
http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/pcm/pcm.c;h=74d1d1a...
So what's it now? It makes more sense to me if it would be a single fd.
No, it's a bug in test/pcm.c. As documented, the API converts (fixes) each poll_fd in the given array.
Hmm.. just looking through the whole tree again, this thing seems mixed up weirdly right now. At least, the latest pcm_hw.c assumes the single revent. Meanwhile, the old pcm_multi.c assumed the multiple revents.
As you notice the work "revents", the function itself supposes the multiple events. However, obviously, handling the single revent is much easier for applications.
So I believe we should go to the current pcm_hw.c implementation way, i.e. demangling to a single revent. But, yes, more investigations and fixes are needed.
And the patch is below. If no one has objection, I'll apply it later.
thanks,
Takashi
== From fdd79369b24d2e9405d4ec5f1bdfc09dda127ed4 Mon Sep 17 00:00:00 2001 From: Takashi Iwai tiwai@suse.de Date: Mon, 2 Feb 2009 15:43:48 +0100 Subject: [PATCH] Fix handling of revents in snd_pcm_poll_descriptors_revents()
The revents parameter is ambiguously defined whether it's a pointer to a single event or an arary. While the recent pcm_hw.c assumes it being a single event, the old multi plugin supposed it as an array. Similarly snd_pcm_wait_nocheck() checks the revents as an arary.
This patch defines the behavior of revents more strictly. It's a pointer of a single event.
Fixed snd_pcm_wait_nocheck() to follow that.
Signed-off-by: Takashi Iwai tiwai@suse.de --- src/pcm/pcm.c | 42 +++++++++++++++++++----------------------- 1 files changed, 19 insertions(+), 23 deletions(-)
diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c index 74d1d1a..0203720 100644 --- a/src/pcm/pcm.c +++ b/src/pcm/pcm.c @@ -1430,7 +1430,7 @@ int snd_pcm_poll_descriptors(snd_pcm_t *pcm, struct pollfd *pfds, unsigned int s * \param pcm PCM handle * \param pfds array of poll descriptors * \param nfds count of poll descriptors - * \param revents returned events + * \param revents pointer to the returned (single) event * \return zero if success, otherwise a negative error code * * This function does "demangling" of the revents mask returned from @@ -1440,6 +1440,9 @@ int snd_pcm_poll_descriptors(snd_pcm_t *pcm, struct pollfd *pfds, unsigned int s * syscall returned that some events are waiting, this function might * return empty set of events. In this case, application should * do next event waiting using poll() or select(). + * + * Note: Even if multiple poll descriptors are used (i.e. pfds > 1), + * this function returns only a single event. */ int snd_pcm_poll_descriptors_revents(snd_pcm_t *pcm, struct pollfd *pfds, unsigned int nfds, unsigned short *revents) { @@ -2338,8 +2341,8 @@ int snd_pcm_wait(snd_pcm_t *pcm, int timeout) int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout) { struct pollfd *pfd; - unsigned short *revents; - int i, npfds, pollio, err, err_poll; + unsigned short revents = 0; + int i, npfds, err, err_poll; npfds = snd_pcm_poll_descriptors_count(pcm); if (npfds <= 0 || npfds >= 16) { @@ -2347,7 +2350,6 @@ int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout) return -EIO; } pfd = alloca(sizeof(*pfd) * npfds); - revents = alloca(sizeof(*revents) * npfds); err = snd_pcm_poll_descriptors(pcm, pfd, npfds); if (err < 0) return err; @@ -2356,7 +2358,6 @@ int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout) return -EIO; } do { - pollio = 0; err_poll = poll(pfd, npfds, timeout); if (err_poll < 0) { if (errno == EINTR) @@ -2365,28 +2366,23 @@ int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout) } if (! err_poll) break; - err = snd_pcm_poll_descriptors_revents(pcm, pfd, npfds, revents); + err = snd_pcm_poll_descriptors_revents(pcm, pfd, npfds, &revents); if (err < 0) return err; - for (i = 0; i < npfds; i++) { - if (revents[i] & (POLLERR | POLLNVAL)) { - /* check more precisely */ - switch (snd_pcm_state(pcm)) { - case SND_PCM_STATE_XRUN: - return -EPIPE; - case SND_PCM_STATE_SUSPENDED: - return -ESTRPIPE; - case SND_PCM_STATE_DISCONNECTED: - return -ENODEV; - default: - return -EIO; - } + if (revents & (POLLERR | POLLNVAL)) { + /* check more precisely */ + switch (snd_pcm_state(pcm)) { + case SND_PCM_STATE_XRUN: + return -EPIPE; + case SND_PCM_STATE_SUSPENDED: + return -ESTRPIPE; + case SND_PCM_STATE_DISCONNECTED: + return -ENODEV; + default: + return -EIO; } - if ((revents[i] & (POLLIN | POLLOUT)) == 0) - continue; - pollio++; } - } while (! pollio); + } while (!(revents & (POLLIN | POLLOUT))); #if 0 /* very useful code to test poll related problems */ { snd_pcm_sframes_t avail_update;