[v2,12/24] Convert message writing from C style to C++ style

Submitted by Christophe de Dinechin on Feb. 21, 2018, 5:46 p.m.

Details

Message ID 20180221174636.37758-13-christophe@dinechin.org
State New
Headers show
Series "WIP: Refactor the streaming agent towards a more standard C++ style" ( rev: 2 ) in Spice

Not browsing as part of any series.

Commit Message

Christophe de Dinechin Feb. 21, 2018, 5:46 p.m.
From: Christophe de Dinechin <dinechin@redhat.com>

- The Stream class now deals with locking and sending messages
- The Message<> template class deals with the general writing mechanisms
- Three classes, FormatMessage, FrameMessage and X11CursorMessage represent individual messages

The various classes should be moved to separate headers in a subsequent operation

The design uses templates to avoid any runtime overhead. All the calls are static.

Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
---
 src/spice-streaming-agent.cpp | 255 ++++++++++++++++++++----------------------
 1 file changed, 122 insertions(+), 133 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
index 9f4183c..85d055b 100644
--- a/src/spice-streaming-agent.cpp
+++ b/src/spice-streaming-agent.cpp
@@ -48,24 +48,6 @@  namespace spice
 namespace streaming_agent
 {
 
-struct FormatMessage
-{
-    StreamDevHeader hdr;
-    StreamMsgFormat msg;
-};
-
-struct DataMessage
-{
-    StreamDevHeader hdr;
-    StreamMsgData msg;
-};
-
-struct CursorMessage
-{
-    StreamDevHeader hdr;
-    StreamMsgCursorSet msg;
-};
-
 class Stream
 {
 public:
@@ -79,7 +61,6 @@  public:
     {
         close(streamfd);
     }
-    operator int() { return streamfd; }
 
     int have_something_to_read(int timeout);
     void handle_stream_start_stop(uint32_t len);
@@ -88,18 +69,131 @@  public:
     void read_command_from_device(void);
     int read_command(bool blocking);
 
+
+    template <typename Message, typename ...PayloadArgs>
+    bool send(PayloadArgs... payload)
+    {
+        Message message(payload...);
+        std::lock_guard<std::mutex> stream_guard(mutex);
+        size_t expected = message.size(payload...);
+        size_t written = message.write(*this, payload...);
+        return written == expected;
+    }
+
     size_t write_all(const void *buf, const size_t len);
-    int send_format(unsigned w, unsigned h, uint8_t c);
-    int send_frame(const void *buf, const unsigned size);
-    void send_cursor(uint16_t width, uint16_t height,
-                     uint16_t hotspot_x, uint16_t hotspot_y,
-                     std::function<void(uint32_t *)> fill_cursor);
 
 private:
     int streamfd = -1;
     std::mutex mutex;
 };
 
+
+template <typename Payload, typename Info>
+struct Message
+{
+    template <typename ...PayloadArgs>
+    Message(PayloadArgs... payload)
+        : hdr(StreamDevHeader {
+              .protocol_version = STREAM_DEVICE_PROTOCOL,
+              .padding = 0,     // Workaround GCC bug "sorry: not implemented"
+              .type = Info::type,
+              .size = (uint32_t) (Info::size(payload...) - sizeof(hdr))
+          }),
+          msg(Info::make(payload...))
+    { }
+
+    StreamDevHeader hdr;
+    Payload msg;
+    typedef Message message_t;  // Used to compute sizes easily
+};
+
+
+struct FormatMessage : Message<StreamMsgFormat, FormatMessage>
+{
+    FormatMessage(unsigned w, unsigned h, uint8_t c) : Message(w, h, c) {}
+    static const StreamMsgType type = STREAM_TYPE_FORMAT;
+    static size_t size(unsigned width, unsigned height, uint8_t codec)
+    {
+        return sizeof(message_t);
+    }
+    static StreamMsgFormat make(unsigned w, unsigned h, uint8_t c)
+    {
+        return StreamMsgFormat{ .width = w, .height = h, .codec = c, .padding1 = {} };
+    }
+    size_t write(Stream &stream, unsigned w, unsigned h, uint8_t c)
+    {
+        return stream.write_all(this, sizeof(message_t));
+    }
+};
+
+
+struct FrameMessage : Message<StreamMsgData, FrameMessage>
+{
+    FrameMessage(const void *frame, size_t length) : Message(frame, length) {}
+    static const StreamMsgType type = STREAM_TYPE_DATA;
+    static size_t size(const void *frame, size_t length)
+    {
+        return sizeof(message_t) + length;
+    }
+    static StreamMsgData make(const void *frame, size_t length)
+    {
+        return StreamMsgData();
+    }
+    size_t write(Stream &stream, const void *frame, size_t length)
+    {
+        return stream.write_all(this, sizeof(message_t))
+            +  stream.write_all(frame, length);
+    }
+};
+
+struct X11CursorMessage : Message<StreamMsgCursorSet, X11CursorMessage>
+{
+    X11CursorMessage(XFixesCursorImage *cursor)
+        : Message(cursor),
+          pixels(new uint32_t[pixel_count(cursor)])
+    {
+        fill_pixels(cursor);
+    }
+    static const StreamMsgType type = STREAM_TYPE_CURSOR_SET;
+    static size_t pixel_count(XFixesCursorImage *cursor)
+    {
+        return cursor->width * cursor->height;
+    }
+    static size_t size(XFixesCursorImage *cursor)
+    {
+        return sizeof(message_t) + sizeof(uint32_t) * pixel_count(cursor);
+    }
+    static StreamMsgCursorSet make(XFixesCursorImage *cursor)
+    {
+        return StreamMsgCursorSet
+        {
+            .width = cursor->width,
+            .height = cursor->height,
+            .hot_spot_x = cursor->xhot,
+            .hot_spot_y = cursor->yhot,
+            .type = SPICE_CURSOR_TYPE_ALPHA,
+            .padding1 = { },
+            .data = { }
+        };
+    }
+    size_t write(Stream &stream, XFixesCursorImage *cursor)
+    {
+        unsigned pixel_size = pixel_count(cursor) * sizeof(uint32_t);
+        return stream.write_all(this, sizeof(message_t))
+            +  stream.write_all(pixels.get(), pixel_size);
+    }
+    void fill_pixels(XFixesCursorImage *cursor)
+    {
+        uint32_t *pixbuf = pixels.get();
+        unsigned count = pixel_count(cursor);
+        for (unsigned i = 0; i < count; ++i) {
+            pixbuf[i] = cursor->pixels[i];
+        }
+    }
+private:
+    std::unique_ptr<uint32_t[]> pixels;
+};
+
 }} // namespace spice::streaming_agent
 
 
@@ -240,66 +334,6 @@  size_t Stream::write_all(const void *buf, const size_t len)
     return written;
 }
 
-int Stream::send_format(unsigned w, unsigned h, uint8_t c)
-{
-    const size_t msgsize = sizeof(FormatMessage);
-    const size_t hdrsize  = sizeof(StreamDevHeader);
-    FormatMessage msg = {
-        .hdr = {
-            .protocol_version = STREAM_DEVICE_PROTOCOL,
-            .padding = 0,       // Workaround GCC "not implemented" bug
-            .type = STREAM_TYPE_FORMAT,
-            .size = msgsize - hdrsize
-        },
-        .msg = {
-            .width = w,
-            .height = h,
-            .codec = c,
-            .padding1 = { }
-        }
-    };
-    syslog(LOG_DEBUG, "writing format\n");
-    std::lock_guard<std::mutex> stream_guard(mutex);
-    if (write_all(&msg, msgsize) != msgsize) {
-        return EXIT_FAILURE;
-    }
-    return EXIT_SUCCESS;
-}
-
-int Stream::send_frame(const void *buf, const unsigned size)
-{
-    ssize_t n;
-    const size_t msgsize = sizeof(FormatMessage);
-    DataMessage msg = {
-        .hdr = {
-            .protocol_version = STREAM_DEVICE_PROTOCOL,
-            .padding = 0,       // Workaround GCC "not implemented" bug
-            .type = STREAM_TYPE_DATA,
-            .size = size  /* includes only the body? */
-        },
-        .msg = {}
-    };
-
-    std::lock_guard<std::mutex> stream_guard(mutex);
-    n = write_all(&msg, msgsize);
-    syslog(LOG_DEBUG,
-           "wrote %ld bytes of header of data msg with frame of size %u bytes\n",
-           n, msg.hdr.size);
-    if (n != msgsize) {
-        syslog(LOG_WARNING, "write_all header: wrote %ld expected %lu\n",
-               n, msgsize);
-        return EXIT_FAILURE;
-    }
-    n = write_all(buf, size);
-    syslog(LOG_DEBUG, "wrote data msg body of size %ld\n", n);
-    if (n != size) {
-        syslog(LOG_WARNING, "write_all header: wrote %ld expected %u\n",
-               n, size);
-        return EXIT_FAILURE;
-    }
-    return EXIT_SUCCESS;
-}
-
 /* returns current time in micro-seconds */
 static uint64_t get_time(void)
 {
@@ -343,47 +377,6 @@  static void usage(const char *progname)
     exit(1);
 }
 
-void
-Stream::send_cursor(uint16_t width, uint16_t height,
-                    uint16_t hotspot_x, uint16_t hotspot_y,
-                    std::function<void(uint32_t *)> fill_cursor)
-{
-    if (width >= STREAM_MSG_CURSOR_SET_MAX_WIDTH ||
-        height >= STREAM_MSG_CURSOR_SET_MAX_HEIGHT)
-        return;
-
-    const uint32_t cursor_msgsize =
-        sizeof(CursorMessage) + width * height * sizeof(uint32_t);
-    const uint32_t hdrsize  = sizeof(StreamDevHeader);
-
-    std::unique_ptr<uint8_t[]> storage(new uint8_t[cursor_msgsize]);
-
-    CursorMessage *cursor_msg =
-        new(storage.get()) CursorMessage {
-        .hdr = {
-            .protocol_version = STREAM_DEVICE_PROTOCOL,
-            .padding = 0,       // Workaround GCC internal / not implemented compiler error
-            .type = STREAM_TYPE_CURSOR_SET,
-            .size = cursor_msgsize - hdrsize
-        },
-        .msg = {
-            .width = width,
-            .height = height,
-            .hot_spot_x = hotspot_x,
-            .hot_spot_y = hotspot_y,
-            .type = SPICE_CURSOR_TYPE_ALPHA,
-            .padding1 = { },
-            .data = { }
-        }
-    };
-
-    uint32_t *pixels = reinterpret_cast<uint32_t *>(cursor_msg->msg.data);
-    fill_cursor(pixels);
-
-    std::lock_guard<std::mutex> stream_guard(mutex);
-    write_all(storage.get(), cursor_msgsize);
-}
-
 static void cursor_changes(Stream *stream, Display *display, int event_base)
 {
     unsigned long last_serial = 0;
@@ -402,12 +395,8 @@  static void cursor_changes(Stream *stream, Display *display, int event_base)
             continue;
 
         last_serial = cursor->cursor_serial;
-        auto fill_cursor = [cursor](uint32_t *pixels) {
-            for (unsigned i = 0; i < cursor->width * cursor->height; ++i)
-                pixels[i] = cursor->pixels[i];
-        };
-        stream->send_cursor(cursor->width, cursor->height,
-                            cursor->xhot, cursor->yhot, fill_cursor);
+        if (!stream->send<X11CursorMessage>(cursor))
+            syslog(LOG_WARNING, "FAILED to send cursor\n");
     }
 }
 
@@ -461,7 +450,7 @@  do_capture(Stream &stream, const char *streamport, FILE *f_log)
 
                 syslog(LOG_DEBUG, "wXh %uX%u  codec=%u\n", width, height, codec);
 
-                if (stream.send_format(width, height, codec) == EXIT_FAILURE)
+                if (!stream.send<FormatMessage>(width, height, codec))
                     throw std::runtime_error("FAILED to send format message");
             }
             if (f_log) {
@@ -473,7 +462,7 @@  do_capture(Stream &stream, const char *streamport, FILE *f_log)
                     hexdump(frame.buffer, frame.buffer_size, f_log);
                 }
             }
-            if (stream.send_frame(frame.buffer, frame.buffer_size) == EXIT_FAILURE) {
+            if (!stream.send<FrameMessage>(frame.buffer, frame.buffer_size)) {
                 syslog(LOG_ERR, "FAILED to send a frame\n");
                 break;
             }