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

[Christophe de Dinechin]

> On 1 Mar 2018, at 16:14, Lukáš Hrázký <lhrazky at redhat.com> wrote:
> 
> 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?

After splitting, it’s used in two locations that are in different .cpp files.

> 
>> +
>> +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;
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel