[Spice-devel] [PATCH spice-streaming-agent 2/9] Move out {read, write}_all() to a separate module

[Frediano Ziglio]

> 
> This starts at the bottom and prepares for gradually moving more stuff
> out of the main file.
> 
> Signed-off-by: Lukáš Hrázký <lhrazky at redhat.com>
> ---
>  src/Makefile.am               |  2 ++
>  src/spice-streaming-agent.cpp | 48 ++++---------------------------------
>  src/stream-port.cpp           | 55
>  +++++++++++++++++++++++++++++++++++++++++++
>  src/stream-port.hpp           | 21 +++++++++++++++++
>  4 files changed, 83 insertions(+), 43 deletions(-)
>  create mode 100644 src/stream-port.cpp
>  create mode 100644 src/stream-port.hpp
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 606f51a..604c1e5 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -56,4 +56,6 @@ spice_streaming_agent_SOURCES = \
>  	mjpeg-fallback.hpp \
>  	jpeg.cpp \
>  	jpeg.hpp \
> +	stream-port.cpp \
> +	stream-port.hpp \
>  	$(NULL)
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index 5dd41a9..7b166d3 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -7,6 +7,7 @@
>  #include "concrete-agent.hpp"
>  #include "hexdump.h"
>  #include "mjpeg-fallback.hpp"
> +#include "stream-port.hpp"
>  
>  #include <spice/stream-device.h>
>  #include <spice/enums.h>
> @@ -40,8 +41,6 @@
>  
>  using namespace spice::streaming_agent;
>  
> -static size_t write_all(int fd, const void *buf, const size_t len);
> -
>  static ConcreteAgent agent;
>  
>  struct SpiceStreamFormatMessage
> @@ -79,24 +78,6 @@ static int have_something_to_read(int timeout)
>      return 0;
>  }
>  
> -static void read_all(void *msg, size_t len)
> -{
> -    while (len > 0) {
> -        ssize_t n = read(streamfd, msg, len);
> -
> -        if (n < 0) {
> -            if (errno == EINTR) {
> -                continue;
> -            }
> -            throw std::runtime_error("Reading message from device failed: "
> +
> -                                     std::string(strerror(errno)));
> -        }
> -
> -        len -= n;
> -        msg = (uint8_t *) msg + n;
> -    }
> -}
> -
>  static void handle_stream_start_stop(uint32_t len)
>  {
>      uint8_t msg[256];
> @@ -106,7 +87,7 @@ static void handle_stream_start_stop(uint32_t len)
>                                   "(longer than " +
>                                   std::to_string(sizeof(msg)) + ")");
>      }
>  
> -    read_all(msg, len);
> +    read_all(streamfd, msg, len);
>      streaming_requested = (msg[0] != 0); /* num_codecs */
>      syslog(LOG_INFO, "GOT START_STOP message -- request to %s streaming\n",
>             streaming_requested ? "START" : "STOP");
> @@ -124,7 +105,7 @@ static void handle_stream_capabilities(uint32_t len)
>          throw std::runtime_error("capability message too long");
>      }
>  
> -    read_all(caps, len);
> +    read_all(streamfd, caps, len);
>      // we currently do not support extensions so just reply so
>      StreamDevHeader hdr = {
>          STREAM_DEVICE_PROTOCOL,
> @@ -151,7 +132,7 @@ static void handle_stream_error(size_t len)
>  
>      size_t len_to_read = std::min(len, sizeof(msg) - 1);
>  
> -    read_all(&msg, len_to_read);
> +    read_all(streamfd, &msg, len_to_read);
>      msg.msg[len_to_read - sizeof(StreamMsgNotifyError)] = '\0';
>  
>      syslog(LOG_ERR, "Received NotifyError message from the server: %d -
>      %s\n",
> @@ -169,7 +150,7 @@ static void read_command_from_device(void)
>  
>      std::lock_guard<std::mutex> stream_guard(stream_mtx);
>  
> -    read_all(&hdr, sizeof(hdr));
> +    read_all(streamfd, &hdr, sizeof(hdr));
>  
>      if (hdr.protocol_version != STREAM_DEVICE_PROTOCOL) {
>          throw std::runtime_error("BAD VERSION " +
>          std::to_string(hdr.protocol_version) +
> @@ -205,25 +186,6 @@ static int read_command(bool blocking)
>      return 1;
>  }
>  
> -static size_t
> -write_all(int fd, const void *buf, const size_t len)
> -{
> -    size_t written = 0;
> -    while (written < len) {
> -        int l = write(fd, (const char *) buf + written, len - written);
> -        if (l < 0) {
> -            if (errno == EINTR) {
> -                continue;
> -            }
> -            syslog(LOG_ERR, "write failed - %m");
> -            return l;
> -        }
> -        written += l;
> -    }
> -    syslog(LOG_DEBUG, "write_all -- %u bytes written\n", (unsigned)written);
> -    return written;
> -}
> -
>  static int spice_stream_send_format(unsigned w, unsigned h, unsigned c)
>  {
>  
> diff --git a/src/stream-port.cpp b/src/stream-port.cpp
> new file mode 100644
> index 0000000..3699d92
> --- /dev/null
> +++ b/src/stream-port.cpp
> @@ -0,0 +1,55 @@
> +/* A module for low-level communication over the streaming virtio port.
> + *
> + * \copyright
> + * Copyright 2018 Red Hat Inc. All rights reserved.
> + */
> +
> +#include "stream-port.hpp"
> +
> +#include <errno.h>
> +#include <string.h>
> +#include <syslog.h>
> +#include <unistd.h>
> +#include <stdexcept>
> +
> +
> +namespace spice {
> +namespace streaming_agent {
> +
> +void read_all(int fd, void *msg, size_t len)
> +{
> +    while (len > 0) {
> +        ssize_t n = read(fd, msg, len);
> +
> +        if (n < 0) {
> +            if (errno == EINTR) {
> +                continue;
> +            }
> +            throw std::runtime_error("Reading message from device failed: "
> +
> +                                     std::string(strerror(errno)));
> +        }
> +
> +        len -= n;
> +        msg = (uint8_t *) msg + n;
> +    }
> +}
> +
> +size_t write_all(int fd, const void *buf, const size_t len)
> +{
> +    size_t written = 0;
> +    while (written < len) {
> +        int l = write(fd, (const char *) buf + written, len - written);
> +        if (l < 0) {
> +            if (errno == EINTR) {
> +                continue;
> +            }
> +            syslog(LOG_ERR, "write failed - %m");
> +            return l;
> +        }
> +        written += l;
> +    }
> +    syslog(LOG_DEBUG, "write_all -- %u bytes written\n", (unsigned)written);
> +    return written;
> +}
> +
> +}} // namespace spice::streaming_agent
> diff --git a/src/stream-port.hpp b/src/stream-port.hpp
> new file mode 100644
> index 0000000..a296a5c
> --- /dev/null
> +++ b/src/stream-port.hpp
> @@ -0,0 +1,21 @@
> +/* A module for low-level communication over the streaming virtio port.
> + *
> + * \copyright
> + * Copyright 2018 Red Hat Inc. All rights reserved.
> + */
> +
> +#ifndef SPICE_STREAMING_AGENT_STREAM_PORT_HPP
> +#define SPICE_STREAMING_AGENT_STREAM_PORT_HPP
> +
> +#include <cstddef>
> +
> +
> +namespace spice {
> +namespace streaming_agent {
> +
> +void read_all(int fd, void *msg, size_t len);
> +size_t write_all(int fd, const void *buf, const size_t len);
> +
> +}} // namespace spice::streaming_agent
> +
> +#endif // SPICE_STREAMING_AGENT_STREAM_PORT_HPP

Acked-by: Frediano Ziglio <fziglio at redhat.com>

Frediano