[Spice-devel] [PATCH spice-streaming-agent v2 9/9] Refactor and simplify polling for messages to read

[Frediano Ziglio]

> 
> Use exceptions for errors, remove the inner loop in read_command, which
> should practically never do more than one iteration. Handle EINTR from
> poll and report nothing to read, relying on the enclosing loop to poll
> for the command on the next iteration.
> 
> Signed-off-by: Lukáš Hrázký <lhrazky at redhat.com>
> ---
>  src/spice-streaming-agent.cpp | 40 ++++++++++++++--------------------------
>  1 file changed, 14 insertions(+), 26 deletions(-)
> 
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index a9e5bf5..806da25 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -60,20 +60,24 @@ static bool quit_requested = false;
>  static bool log_binary = false;
>  static std::set<SpiceVideoCodecType> client_codecs;
>  
> -static int have_something_to_read(StreamPort &stream_port, int timeout)
> +static bool have_something_to_read(StreamPort &stream_port, bool blocking)
>  {
>      struct pollfd pollfd = {stream_port.fd, POLLIN, 0};
>  
> -    if (poll(&pollfd, 1, timeout) < 0) {
> -        syslog(LOG_ERR, "poll FAILED\n");
> -        return -1;
> +    if (poll(&pollfd, 1, blocking ? -1 : 0) < 0) {
> +        if (errno == EINTR) {
> +            // report nothing to read, next iteration of the enclosing loop
> will retry
> +            return false;
> +        }
> +
> +        throw IOError("poll failed on the device", errno);
>      }
>  
>      if (pollfd.revents & POLLIN) {
> -        return 1;
> +        return true;
>      }
>  
> -    return 0;
> +    return false;
>  }
>  
>  static void handle_stream_start_stop(StreamPort &stream_port, uint32_t len)
> @@ -164,22 +168,11 @@ static void read_command_from_device(StreamPort
> &stream_port)
>      throw std::runtime_error("UNKNOWN msg of type " +
>      std::to_string(hdr.type));
>  }
>  
> -static int read_command(StreamPort &stream_port, bool blocking)
> +static void read_command(StreamPort &stream_port, bool blocking)
>  {
> -    int timeout = blocking?-1:0;
> -    while (!quit_requested) {
> -        if (!have_something_to_read(stream_port, timeout)) {
> -            if (!blocking) {
> -                return 0;
> -            }
> -            sleep(1);
> -            continue;
> -        }
> +    if (have_something_to_read(stream_port, blocking)) {

Why you removed the while?
If there are multiple commands to read potentially only one
will be read.
Yes, maybe should be called read_messages instead.

>          read_command_from_device(stream_port);
> -        break;
>      }
> -
> -    return 1;
>  }
>  
>  static void spice_stream_send_format(StreamPort &stream_port, unsigned w,
>  unsigned h, unsigned c)
> @@ -332,9 +325,7 @@ do_capture(StreamPort &stream_port, FILE *f_log)
>      unsigned int frame_count = 0;
>      while (!quit_requested) {
>          while (!quit_requested && !streaming_requested) {
> -            if (read_command(stream_port, true) < 0) {
> -                throw std::runtime_error("FAILED to read command");
> -            }
> +            read_command(stream_port, true);
>          }
>  
>          if (quit_requested) {
> @@ -395,10 +386,7 @@ do_capture(StreamPort &stream_port, FILE *f_log)
>                  break;
>              }
>  
> -            //usleep(1);
> -            if (read_command(stream_port, false) < 0) {
> -                throw std::runtime_error("FAILED to read command");
> -            }
> +            read_command(stream_port, false);
>          }
>      }
>  }

Frediano