[Spice-devel] [PATCH 15/22] Create FrameLog class to encapsulate logging of frames

[Christophe de Dinechin]

From: Christophe de Dinechin <dinechin at redhat.com>

The FrameLog encapsulates the frame binary log, making sure that the
log is properly closed in case of exceptions.

Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>
---
 include/spice-streaming-agent/errors.hpp | 10 ++++
 src/errors.cpp                           |  6 ++
 src/spice-streaming-agent.cpp            | 94 +++++++++++++++++++-------------
 3 files changed, 72 insertions(+), 38 deletions(-)

diff --git a/include/spice-streaming-agent/errors.hpp b/include/spice-streaming-agent/errors.hpp
index ca70d2e..72e9910 100644
--- a/include/spice-streaming-agent/errors.hpp
+++ b/include/spice-streaming-agent/errors.hpp
@@ -34,6 +34,16 @@ protected:
     int saved_errno;
 };
 
+class OpenError : public IOError
+{
+public:
+    OpenError(const char *msg, const char *filename, int saved_errno)
+        : IOError(msg, saved_errno), filename(filename) {}
+    int format_message(char *buffer, size_t size) const noexcept override;
+protected:
+    const char *filename;
+};
+
 class WriteError : public IOError
 {
 public:
diff --git a/src/errors.cpp b/src/errors.cpp
index 01bb162..ff25142 100644
--- a/src/errors.cpp
+++ b/src/errors.cpp
@@ -47,6 +47,12 @@ int IOError::append_strerror(char *buffer, size_t size, int written) const noexc
     return written;
 }
 
+int OpenError::format_message(char *buffer, size_t size) const noexcept
+{
+    int written = snprintf(buffer, size, "%s '%s'", what(), filename);
+    return append_strerror(buffer, size, written);
+}
+
 int WriteError::format_message(char *buffer, size_t size) const noexcept
 {
     int written = snprintf(buffer, size, "%s writing %s", what(), operation);
diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
index d1996fd..9e643bf 100644
--- a/src/spice-streaming-agent.cpp
+++ b/src/spice-streaming-agent.cpp
@@ -48,6 +48,17 @@ namespace spice
 namespace streaming_agent
 {
 
+/* returns current time in micro-seconds */
+static uint64_t get_time(void)
+{
+    struct timeval now;
+
+    gettimeofday(&now, NULL);
+
+    return (uint64_t)now.tv_sec * 1000000 + (uint64_t)now.tv_usec;
+
+}
+
 class Stream
 {
     typedef std::set<SpiceVideoCodecType> codecs_t;
@@ -218,6 +229,45 @@ private:
     Display *display;
 };
 
+class FrameLog
+{
+public:
+    FrameLog(const char *filename, bool binary = false);
+    ~FrameLog();
+
+    operator bool() { return log != NULL; }
+    void dump(const void *buffer, size_t length);
+
+private:
+    FILE *log;
+    bool binary;
+};
+
+
+FrameLog::FrameLog(const char *filename, bool binary)
+    : log(filename ? fopen(filename, "wb") : NULL), binary(binary)
+{
+    if (filename && !log) {
+        throw OpenError("failed to open hexdump log file", filename, errno);
+    }
+}
+
+FrameLog::~FrameLog()
+{
+    if (log)
+        fclose(log);
+}
+
+void FrameLog::dump(const void *buffer, size_t length)
+{
+    if (binary) {
+        fwrite(buffer, length, 1, log);
+    } else {
+        fprintf(log, "%" PRIu64 ": Frame of %zu bytes:\n", get_time(), length);
+        hexdump(buffer, length, log);
+    }
+}
+
 class X11CursorThread
 {
 public:
@@ -279,11 +329,9 @@ X11CursorThread::X11CursorThread(Stream &stream)
     thread.detach();
 }
 
-
 }} // namespace spice::streaming_agent
 
 static bool quit_requested = false;
-static bool log_binary = false;
 
 int Stream::have_something_to_read(int timeout)
 {
@@ -407,17 +455,6 @@ void Stream::write_all(const char *operation, const void *buf, const size_t len)
     syslog(LOG_DEBUG, "write_all -- %zu bytes written\n", written);
 }
 
-/* returns current time in micro-seconds */
-static uint64_t get_time(void)
-{
-    struct timeval now;
-
-    gettimeofday(&now, NULL);
-
-    return (uint64_t)now.tv_sec * 1000000 + (uint64_t)now.tv_usec;
-
-}
-
 static void handle_interrupt(int intr)
 {
     syslog(LOG_INFO, "Got signal %d, exiting", intr);
@@ -452,7 +489,7 @@ static void usage(const char *progname)
 }
 
 static void
-do_capture(Stream &stream, const char *streamport, FILE *f_log)
+do_capture(Stream &stream, const char *streamport, FrameLog &frame_log)
 {
     unsigned int frame_count = 0;
     while (!quit_requested) {
@@ -504,14 +541,8 @@ do_capture(Stream &stream, const char *streamport, FILE *f_log)
 
                 stream.send<FormatMessage>(width, height, codec);
             }
-            if (f_log) {
-                if (log_binary) {
-                    fwrite(frame.buffer, frame.buffer_size, 1, f_log);
-                } else {
-                    fprintf(f_log, "%" PRIu64 ": Frame of %zu bytes:\n",
-                            get_time(), frame.buffer_size);
-                    hexdump(frame.buffer, frame.buffer_size, f_log);
-                }
+            if (frame_log) {
+                frame_log.dump(frame.buffer, frame.buffer_size);
             }
             stream.send<FrameMessage>(frame.buffer, frame.buffer_size);
             //usleep(1);
@@ -530,6 +561,7 @@ int main(int argc, char* argv[])
     const char *streamport = "/dev/virtio-ports/com.redhat.stream.0";
     int opt;
     const char *log_filename = NULL;
+    bool log_binary = false;
     int logmask = LOG_UPTO(LOG_WARNING);
     const char *pluginsdir = PLUGINSDIR;
     enum {
@@ -593,21 +625,12 @@ int main(int argc, char* argv[])
 
     register_interrupts();
 
-    FILE *f_log = NULL;
-    if (log_filename) {
-        f_log = fopen(log_filename, "wb");
-        if (!f_log) {
-            syslog(LOG_ERR, "Failed to open log file '%s': %s\n",
-                   log_filename, strerror(errno));
-            return EXIT_FAILURE;
-        }
-    }
-
     int ret = EXIT_SUCCESS;
     try {
         Stream stream(streamport);
+        FrameLog frame_log(log_filename, log_binary);
         X11CursorThread cursor_thread(stream);
-        do_capture(stream, streamport, f_log);
+        do_capture(stream, streamport, frame_log);
     }
     catch (Error &err) {
         err.syslog();
@@ -617,11 +640,6 @@ int main(int argc, char* argv[])
         syslog(LOG_ERR, "%s\n", err.what());
         ret = EXIT_FAILURE;
     }
-
-    if (f_log) {
-        fclose(f_log);
-        f_log = NULL;
-    }
     closelog();
     return ret;
 }
-- 
2.13.5 (Apple Git-94)