On Thu, 2018-03-01 at 16:47 +0100, Christophe de Dinechin wrote:
> > On 1 Mar 2018, at 16:14, Lukáš Hrázký <lhrazky@xxxxxxxxxx> wrote:
> >
> > On Wed, 2018-02-28 at 16:43 +0100, Christophe de Dinechin wrote:
> > > From: Christophe de Dinechin <dinechin@xxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxx>
> > > ---
> > > 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.
Well, the definition should be moved to a .cpp file and it seems to me
then this function belongs into somethings like utils.hpp.
Also (more likely a followup, unless it would spare us creating a
utils.hpp?), std::crono could be used? I need to study it a bit, not
too familiar with it, so I am not sure (Frediano mentioned to me some
problem with that which I forgot).
(I am also fine leaving it as-is and fixing it in a followup
alltogether, as I can see some things snowballing for you and you need
to stop somewhere.)
> >
> > > +
> > > +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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel