core, modules: Remove useless EINTR tests

Submitted by Frédéric Danis on May 28, 2019, 2:49 p.m.

Details

Message ID 20190528144919.1601-1-frederic.danis@collabora.com
State New
Headers show
Series "core, modules: Remove useless EINTR tests" ( rev: 1 ) in PulseAudio

Not browsing as part of any series.

Commit Message

Frédéric Danis May 28, 2019, 2:49 p.m.
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(-)

Patch hide | download patch | download mbox

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;
 
diff --git a/src/modules/module-esound-sink.c b/src/modules/module-esound-sink.c
index 5ff04516a..f46dc3889 100644
--- a/src/modules/module-esound-sink.c
+++ b/src/modules/module-esound-sink.c
@@ -249,9 +249,7 @@  static void thread_func(void *userdata) {
 
                     if (l < 0) {
 
-                        if (errno == EINTR)
-                            continue;
-                        else if (errno == EAGAIN) {
+                        if (errno == EAGAIN) {
 
                             /* OK, we filled all socket buffers up
                              * now. */
diff --git a/src/modules/module-pipe-sink.c b/src/modules/module-pipe-sink.c
index 213924fdc..43595420f 100644
--- a/src/modules/module-pipe-sink.c
+++ b/src/modules/module-pipe-sink.c
@@ -199,14 +199,13 @@  static ssize_t pipe_sink_write(struct userdata *u, pa_memchunk *pchunk) {
         if (l < 0) {
             if (errno == EAGAIN)
                 break;
-            else if (errno != EINTR) {
-                if (!u->fifo_error) {
-                    pa_log("Failed to write data to FIFO: %s", pa_cstrerror(errno));
-                    u->fifo_error = true;
-                }
-                count = -1 - count;
-                break;
+
+            if (!u->fifo_error) {
+                pa_log("Failed to write data to FIFO: %s", pa_cstrerror(errno));
+                u->fifo_error = true;
             }
+            count = -1 - count;
+            break;
         } else {
             if (u->fifo_error) {
                 pa_log_debug("Recovered from FIFO error");
@@ -288,9 +287,7 @@  static int process_render(struct userdata *u) {
 
         if (l < 0) {
 
-            if (errno == EINTR)
-                continue;
-            else if (errno == EAGAIN)
+            if (errno == EAGAIN)
                 return 0;
             else {
                 pa_log("Failed to write data to FIFO: %s", pa_cstrerror(errno));
diff --git a/src/modules/module-pipe-source.c b/src/modules/module-pipe-source.c
index 74ec0551a..32b35c163 100644
--- a/src/modules/module-pipe-source.c
+++ b/src/modules/module-pipe-source.c
@@ -155,9 +155,7 @@  static void thread_func(void *userdata) {
 
             if (l < 0) {
 
-                if (errno == EINTR)
-                    continue;
-                else if (errno != EAGAIN) {
+                if (errno != EAGAIN) {
                     pa_log("Failed to read data from FIFO: %s", pa_cstrerror(errno));
                     goto fail;
                 }
diff --git a/src/modules/module-solaris.c b/src/modules/module-solaris.c
index 038aca114..ec9eb875f 100644
--- a/src/modules/module-solaris.c
+++ b/src/modules/module-solaris.c
@@ -714,9 +714,7 @@  static void thread_func(void *userdata) {
                 pa_memblock_release(u->memchunk.memblock);
 
                 if (w <= 0) {
-                    if (errno == EINTR) {
-                        continue;
-                    } else if (errno == EAGAIN) {
+                    if (errno == EAGAIN) {
                         /* We may have realtime priority so yield the CPU to ensure that fd can become writable again. */
                         pa_log_debug("EAGAIN with %llu bytes buffered.", buffered_bytes);
                         break;
diff --git a/src/modules/oss/module-oss.c b/src/modules/oss/module-oss.c
index ed124cab4..6eb025489 100644
--- a/src/modules/oss/module-oss.c
+++ b/src/modules/oss/module-oss.c
@@ -980,10 +980,7 @@  static void thread_func(void *userdata) {
 
                     if (t < 0) {
 
-                        if (errno == EINTR)
-                            continue;
-
-                        else if (errno == EAGAIN) {
+                        if (errno == EAGAIN) {
                             pa_log_debug("EAGAIN");
 
                             revents &= ~POLLOUT;
@@ -1087,10 +1084,7 @@  static void thread_func(void *userdata) {
                     if (t < 0) {
                         pa_memblock_unref(memchunk.memblock);
 
-                        if (errno == EINTR)
-                            continue;
-
-                        else if (errno == EAGAIN) {
+                        if (errno == EAGAIN) {
                             pa_log_debug("EAGAIN");
 
                             revents &= ~POLLIN;
diff --git a/src/pulsecore/fdsem.c b/src/pulsecore/fdsem.c
index a7fbf95d2..b2f1e7dfc 100644
--- a/src/pulsecore/fdsem.c
+++ b/src/pulsecore/fdsem.c
@@ -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;
             }
@@ -164,11 +161,8 @@  static void flush(pa_fdsem *f) {
 #endif
 
         if ((r = pa_read(f->fds[0], &x, sizeof(x), NULL)) <= 0) {
-
-            if (r >= 0 || errno != EINTR) {
-                pa_log_error("Invalid read from pipe: %s", r < 0 ? pa_cstrerror(errno) : "EOF");
-                pa_assert_not_reached();
-            }
+            pa_log_error("Invalid read from pipe: %s", r < 0 ? pa_cstrerror(errno) : "EOF");
+            pa_assert_not_reached();
 
             continue;
         }
@@ -194,10 +188,8 @@  void pa_fdsem_post(pa_fdsem *f) {
                     uint64_t u = 1;
 
                     if ((r = pa_write(f->efd, &u, sizeof(u), &f->write_type)) != sizeof(u)) {
-                        if (r >= 0 || errno != EINTR) {
-                            pa_log_error("Invalid write to eventfd: %s", r < 0 ? pa_cstrerror(errno) : "EOF");
-                            pa_assert_not_reached();
-                        }
+                        pa_log_error("Invalid write to eventfd: %s", r < 0 ? pa_cstrerror(errno) : "EOF");
+                        pa_assert_not_reached();
 
                         continue;
                     }
@@ -205,10 +197,8 @@  void pa_fdsem_post(pa_fdsem *f) {
 #endif
 
                 if ((r = pa_write(f->fds[1], &x, 1, &f->write_type)) != 1) {
-                    if (r >= 0 || errno != EINTR) {
-                        pa_log_error("Invalid write to pipe: %s", r < 0 ? pa_cstrerror(errno) : "EOF");
-                        pa_assert_not_reached();
-                    }
+                    pa_log_error("Invalid write to pipe: %s", r < 0 ? pa_cstrerror(errno) : "EOF");
+                    pa_assert_not_reached();
 
                     continue;
                 }
@@ -238,11 +228,8 @@  void pa_fdsem_wait(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;
             }
@@ -252,11 +239,8 @@  void pa_fdsem_wait(pa_fdsem *f) {
 #endif
 
         if ((r = pa_read(f->fds[0], &x, sizeof(x), NULL)) <= 0) {
-
-            if (r >= 0 || errno != EINTR) {
-                pa_log_error("Invalid read from pipe: %s", r < 0 ? pa_cstrerror(errno) : "EOF");
-                pa_assert_not_reached();
-            }
+            pa_log_error("Invalid read from pipe: %s", r < 0 ? pa_cstrerror(errno) : "EOF");
+            pa_assert_not_reached();
 
             continue;
         }
diff --git a/src/pulsecore/iochannel.c b/src/pulsecore/iochannel.c
index e25824b78..eb93176ec 100644
--- a/src/pulsecore/iochannel.c
+++ b/src/pulsecore/iochannel.c
@@ -227,7 +227,7 @@  ssize_t pa_iochannel_write(pa_iochannel*io, const void*data, size_t l) {
         return r; /* Fast path - we almost always successfully write everything */
 
     if (r < 0) {
-        if (errno == EINTR || errno == EAGAIN)
+        if (errno == EAGAIN)
             r = 0;
         else
             return r;
diff --git a/src/pulsecore/protocol-esound.c b/src/pulsecore/protocol-esound.c
index d54c7f845..cf0fe4fdf 100644
--- a/src/pulsecore/protocol-esound.c
+++ b/src/pulsecore/protocol-esound.c
@@ -1010,7 +1010,7 @@  static int do_read(connection *c) {
                                    ((uint8_t*) &c->request) + c->read_data_length,
                                    sizeof(c->request) - c->read_data_length)) <= 0) {
 
-            if (r < 0 && (errno == EINTR || errno == EAGAIN))
+            if (r < 0 && errno == EAGAIN)
                 return 0;
 
             pa_log_debug("read(): %s", r < 0 ? pa_cstrerror(errno) : "EOF");
@@ -1066,7 +1066,7 @@  static int do_read(connection *c) {
                                    (uint8_t*) c->read_data + c->read_data_length,
                                    handler->data_length - c->read_data_length)) <= 0) {
 
-            if (r < 0 && (errno == EINTR || errno == EAGAIN))
+            if (r < 0 && errno == EAGAIN)
                 return 0;
 
             pa_log_debug("read(): %s", r < 0 ? pa_cstrerror(errno) : "EOF");
@@ -1097,7 +1097,7 @@  static int do_read(connection *c) {
         pa_memblock_release(c->scache.memchunk.memblock);
 
         if (r <= 0) {
-            if (r < 0 && (errno == EINTR || errno == EAGAIN))
+            if (r < 0 && errno == EAGAIN)
                 return 0;
 
             pa_log_debug("read(): %s", r < 0 ? pa_cstrerror(errno) : "EOF");
@@ -1165,7 +1165,7 @@  static int do_read(connection *c) {
 
         if (r <= 0) {
 
-            if (r < 0 && (errno == EINTR || errno == EAGAIN))
+            if (r < 0 && errno == EAGAIN)
                 return 0;
 
             pa_log_debug("read(): %s", r < 0 ? pa_cstrerror(errno) : "EOF");
diff --git a/src/pulsecore/protocol-simple.c b/src/pulsecore/protocol-simple.c
index 77d05398c..ed6a402e7 100644
--- a/src/pulsecore/protocol-simple.c
+++ b/src/pulsecore/protocol-simple.c
@@ -183,7 +183,7 @@  static int do_read(connection *c) {
 
     if (r <= 0) {
 
-        if (r < 0 && (errno == EINTR || errno == EAGAIN))
+        if (r < 0 && errno == EAGAIN)
             return 0;
 
         pa_log_debug("read(): %s", r == 0 ? "EOF" : pa_cstrerror(errno));

Comments

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.