[Spice-devel] [PATCH v2 00/24] WIP: Refactor the streaming agent towards a more standard C++ style

Christophe de Dinechin christophe at dinechin.org
Wed Feb 21 17:46:12 UTC 2018


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

The streaming agent started as C code. This series converts
the style to something that is more usual for C++, notably:

- Adds encapsulation and RAII for resources such as the stream
- Splits functionality into several classes with clear roles
- Puts message formatting and writing in a reused based class
- Isolate what's specific to each message in three derived classes
- Isolate X11-specific code in separate classes, one for cursor messages,
  one for the thread polling the data.

Changes since v1:

- Applied three different fixes to the X11Cursor case, one seen by Lukas,
  one seen by Frediano, one seen by me.

- This prompted me to decide that the way I was sending the packets
  was insufficiently clear (not robust to copy-pasting), so I
  addressed that.

- Switch to exceptions to report write errors, on a suggestion of
  Lukash. I used that as an opportunity to show what I had in mind
  with respect to "delayed formatting" of exceptions.

- Added a 'catch all' clause at the top, and catch all std::exception,
  so that we don't go to 'terminate()' if a plugin throws
  std::bad_alloc or something like that.

- Still WIP, because I could not test (although for a different reason).
  I had some guest X11 configuration issues after installing the P4 card.
  I have been trying to fix it since this morning, and decided to not
  delay the sending of this series despite having not really tested it,
  because a few people are "blocked" on it

Christophe de Dinechin (24):
  Add missing <string> header
  log_binary is really a boolean
  Eliminate signed/unsigned warning
  Do not create std::string for constants
  Use RAII to cleanup stream in case of exception or return
  Replace inefficient C-style initialization with C++-style
  Get rid of C-style memset initializations, use C++ style aggregates
  Use C++ style for cursor message initialization instead of C style
  Reorder headers according to style guide
  Since we use a namespace, simplify name of local classes
  Move read, write and locking into the 'Stream' class
  Convert message writing from C style to C++ style
  Add more meaningful syslog reporting
  Create a class encapsulating the X11 display cursor capture
  Create FrameLog class to abstract logging of frames
  Remove client_codecs global variable, moved inside the 'Stream' class
  Move the capture loop in the ConcreteAgent, get rid of global agent
    variable
  Catch all std::exception derivatives (including std::bad_alloc, not a
    std::bad_alloc)
  Make streaming_requested a method in Stream
  Throw an exception in case we can't write a complete packet.
  Reformat 'if' statments according to style guide
  Throw exception in case of write failure
  Rename 'streamfd' variable to 'stream', no longer a file descriptor
  Rename 'Stream' to 'IOChannel'

 src/concrete-agent.cpp        |   1 +
 src/concrete-agent.hpp        |   4 +
 src/mjpeg-fallback.cpp        |   2 +-
 src/spice-streaming-agent.cpp | 591 +++++++++++++++++++++++++-----------------
 4 files changed, 362 insertions(+), 236 deletions(-)

-- 
2.13.5 (Apple Git-94)



More information about the Spice-devel mailing list