[Spice-devel] [PATCH 21/22] Moving FrameLog into a separate file

[Lukáš Hrázký]

On Wed, 2018-02-28 at 16:43 +0100, Christophe de Dinechin wrote:
> From: Christophe de Dinechin <dinechin at redhat.com>
> 
> Isolating classes in separate files makes parallel builds faster,
> facilitates code reuse and minimizes the chances of patch conflicts.
> 
> Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>
> ---
>  src/Makefile.am               |  2 ++
>  src/frame-log.cpp             | 45 +++++++++++++++++++++++++++++++++++++
>  src/frame-log.hpp             | 43 +++++++++++++++++++++++++++++++++++
>  src/spice-streaming-agent.cpp | 52 +------------------------------------------
>  4 files changed, 91 insertions(+), 51 deletions(-)
>  create mode 100644 src/frame-log.cpp
>  create mode 100644 src/frame-log.hpp
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 4a03e5e..5e36e1f 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -57,6 +57,8 @@ spice_streaming_agent_SOURCES = \
>  	jpeg.hpp \
>  	stream.cpp \
>  	stream.hpp \
> +	frame-log.hpp \
> +	frame-log.cpp \
>  	errors.cpp \
>  	x11-cursor.hpp \
>  	x11-cursor.cpp \
> diff --git a/src/frame-log.cpp b/src/frame-log.cpp
> new file mode 100644
> index 0000000..53751be
> --- /dev/null
> +++ b/src/frame-log.cpp
> @@ -0,0 +1,45 @@
> +/* Class to log frames as they are being streamed
> + *
> + * \copyright
> + * Copyright 2018 Red Hat Inc. All rights reserved.
> + */
> +
> +#include "frame-log.hpp"
> +#include "hexdump.h"
> +
> +#include <spice-streaming-agent/errors.hpp>
> +
> +#include <syslog.h>
> +#include <inttypes.h>
> +#include <errno.h>
> +
> +namespace spice
> +{
> +namespace streaming_agent
> +{
> +
> +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);
> +    }
> +}
> +
> +}} // namespace spice::streaming_agent
> diff --git a/src/frame-log.hpp b/src/frame-log.hpp
> new file mode 100644
> index 0000000..09dbd4a
> --- /dev/null
> +++ b/src/frame-log.hpp
> @@ -0,0 +1,43 @@
> +/* Class to log frames as they are being streamed
> + *
> + * \copyright
> + * Copyright 2018 Red Hat Inc. All rights reserved.
> + */
> +#ifndef SPICE_STREAMING_AGENT_FRAME_LOG_HPP
> +#define SPICE_STREAMING_AGENT_FRAME_LOG_HPP
> +
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <sys/time.h>
> +
> +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;
> +
> +}

Why in the header?

> +
> +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;
> +};
> +
> +}} // namespace spice::streaming_agent
> +
> +#endif // SPICE_STREAMING_AGENT_ERRORS_HPP
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index 2264af2..424db95 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -7,8 +7,8 @@
>  #include "concrete-agent.hpp"
>  #include "stream.hpp"
>  #include "message.hpp"
> +#include "frame-log.hpp"
>  #include "x11-cursor.hpp"
> -#include "hexdump.h"
>  #include "mjpeg-fallback.hpp"
>  
>  #include <spice/stream-device.h>
> @@ -45,17 +45,6 @@ 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 FormatMessage : public Message<StreamMsgFormat, FormatMessage, STREAM_TYPE_FORMAT>
>  {
>  public:
> @@ -85,45 +74,6 @@ public:
>      }
>  };
>  
> -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);
> -    }
> -}
> -
>  }} // namespace spice::streaming_agent
>  
>  bool quit_requested = false;