On Thursday 16 January 2020 17:31:17 Tanu Kaskinen wrote:
> 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.
This is because pa_read() was replaced by recvmsg() in bluetooth code.
So above information about EINTR is not valid for bluetooth code which
reads data, and needs to properly handle EINTR error code.
> 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!
>
--
Pali Rohár
pali.rohar@xxxxxxxxx
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
