Resurrecting this old patch...
On Mon, 2019-06-03 at 15:43 +0200, Pali Rohár wrote:
> On Tuesday 28 May 2019 16:49:19 Frédéric Danis wrote:
> > Since commit ad447d14682 (in 2009) pa_read and pa_write take care of
> > handling EINTR error.
> > So, pa_read, pa_write, pa_iochannel_read and pa_iochannel_write can not
> > exit with errno set to EINTR, and testing it is useless.
> > ---
> > src/modules/bluetooth/module-bluez5-device.c | 16 +-------
> > src/modules/module-esound-sink.c | 4 +-
> > src/modules/module-pipe-sink.c | 17 ++++-----
> > src/modules/module-pipe-source.c | 4 +-
> > src/modules/module-solaris.c | 4 +-
> > src/modules/oss/module-oss.c | 10 +----
> > src/pulsecore/fdsem.c | 40 ++++++--------------
> > src/pulsecore/iochannel.c | 2 +-
> > src/pulsecore/protocol-esound.c | 8 ++--
> > src/pulsecore/protocol-simple.c | 2 +-
> > 10 files changed, 32 insertions(+), 75 deletions(-)
> >
> > diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
> > index 56c96054d..f850a3a41 100644
> > --- a/src/modules/bluetooth/module-bluez5-device.c
> > +++ b/src/modules/bluetooth/module-bluez5-device.c
> > @@ -279,10 +279,6 @@ static int sco_process_render(struct userdata *u) {
> >
> > saved_errno = errno;
> >
> > - if (saved_errno == EINTR)
> > - /* Retry right away if we got interrupted */
> > - continue;
> > -
> > pa_memblock_unref(memchunk.memblock);
> >
> > if (saved_errno == EAGAIN) {
> > @@ -445,11 +441,7 @@ static int a2dp_write_buffer(struct userdata *u, size_t nbytes) {
> >
> > if (l < 0) {
> >
> > - if (errno == EINTR)
> > - /* Retry right away if we got interrupted */
> > - continue;
> > -
> > - else if (errno == EAGAIN) {
> > + if (errno == EAGAIN) {
> > /* Hmm, apparently the socket was not writable, give up for now */
> > pa_log_debug("Got EAGAIN on write() after POLLOUT, probably there is a temporary connection loss.");
> > break;
> > @@ -543,11 +535,7 @@ static int a2dp_process_push(struct userdata *u) {
> >
> > if (l <= 0) {
> >
> > - if (l < 0 && errno == EINTR)
> > - /* Retry right away if we got interrupted */
> > - continue;
> > -
> > - else if (l < 0 && errno == EAGAIN)
> > + if (l < 0 && errno == EAGAIN)
> > /* Hmm, apparently the socket was not readable, give up for now. */
> > break;
> >
>
> Hi! This change conflicts with "Implement reading SO_TIMESTAMP for A2DP
> source" patch as it stops using pa_read() function. For SO_TIMESTAMP is
> needed recvmsg() and then also handling of EINTR/EAGAIN.
Thanks for pointing this out. It would have been easy for me to miss
this, since git rebased the patch without conflicts.
fdsem.c had changes like this:
> @@ -151,11 +151,8 @@ static void flush(pa_fdsem *f) {
> uint64_t u;
>
> if ((r = pa_read(f->efd, &u, sizeof(u), NULL)) != sizeof(u)) {
> -
> - if (r >= 0 || errno != EINTR) {
> - pa_log_error("Invalid read from eventfd: %s", r < 0 ? pa_cstrerror(errno) : "EOF");
> - pa_assert_not_reached();
> - }
> + pa_log_error("Invalid read from eventfd: %s", r < 0 ? pa_cstrerror(errno) : "EOF");
> + pa_assert_not_reached();
>
> continue;
The "continue" statement after the assertion is unnecessary, so I
removed it.
I created an MR for this patch, since we now push all changes through
GitLab:
https://gitlab.freedesktop.org/pulseaudio/pulseaudio/merge_requests/238
I didn't merge the change yet, because we're currently in freeze. I'll
merge it once 14.0 is out.
Thanks for the patch, Frédéric, and sorry for the long delay!
--
Tanu
https://www.patreon.com/tanuk
https://liberapay.com/tanuk
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
