[pulseaudio-discuss] [PATCH] core, modules: Remove useless EINTR tests

[Tanu Kaskinen]

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