[Spice-devel] [PATCH 05/17] Use RAII to cleanup stream in case of exception or return
Christophe de Dinechin
christophe at dinechin.org
Fri Feb 16 16:15:35 UTC 2018
From: Christophe de Dinechin <dinechin at redhat.com>
Get rid of C-style 'goto done' in do_capture.
Get rid of global streamfd, pass it around (cleaned up in later patch)
Fixes a race condition, make sure we only use stream after opening
Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>
---
src/spice-streaming-agent.cpp | 79 +++++++++++++++++++++++++------------------
1 file changed, 47 insertions(+), 32 deletions(-)
diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
index 646d15b..9b8fd45 100644
--- a/src/spice-streaming-agent.cpp
+++ b/src/spice-streaming-agent.cpp
@@ -53,14 +53,38 @@ struct SpiceStreamDataMessage
StreamMsgData msg;
};
+struct SpiceStreamCursorMessage
+{
+ StreamDevHeader hdr;
+ StreamMsgCursorSet msg;
+};
+
+class Stream
+{
+public:
+ Stream(const char *name)
+ {
+ fd = open(name, O_RDWR);
+ if (fd < 0)
+ throw std::runtime_error("failed to open streaming device");
+ }
+ ~Stream()
+ {
+ close(fd);
+ }
+ operator int() { return fd; }
+
+private:
+ int fd = -1;
+};
+
static bool streaming_requested = false;
static bool quit_requested = false;
static bool log_binary = false;
static std::set<SpiceVideoCodecType> client_codecs;
-static int streamfd = -1;
static std::mutex stream_mtx;
-static int have_something_to_read(int timeout)
+static int have_something_to_read(int streamfd, int timeout)
{
struct pollfd pollfd = {streamfd, POLLIN, 0};
@@ -76,7 +100,7 @@ static int have_something_to_read(int timeout)
return 0;
}
-static int read_command_from_device(void)
+static int read_command_from_device(int streamfd)
{
StreamDevHeader hdr;
uint8_t msg[64];
@@ -121,18 +145,18 @@ static int read_command_from_device(void)
return 1;
}
-static int read_command(bool blocking)
+static int read_command(int streamfd, bool blocking)
{
int timeout = blocking?-1:0;
while (!quit_requested) {
- if (!have_something_to_read(timeout)) {
+ if (!have_something_to_read(streamfd, timeout)) {
if (!blocking) {
return 0;
}
sleep(1);
continue;
}
- return read_command_from_device();
+ return read_command_from_device(streamfd);
}
return 1;
@@ -157,7 +181,7 @@ write_all(int fd, const void *buf, const size_t len)
return written;
}
-static int spice_stream_send_format(unsigned w, unsigned h, unsigned c)
+static int spice_stream_send_format(int streamfd, unsigned w, unsigned h, unsigned c)
{
SpiceStreamFormatMessage msg;
@@ -178,7 +202,7 @@ static int spice_stream_send_format(unsigned w, unsigned h, unsigned c)
return EXIT_SUCCESS;
}
-static int spice_stream_send_frame(const void *buf, const unsigned size)
+static int spice_stream_send_frame(int streamfd, const void *buf, const unsigned size)
{
SpiceStreamDataMessage msg;
const size_t msgsize = sizeof(msg);
@@ -254,7 +278,7 @@ static void usage(const char *progname)
}
static void
-send_cursor(unsigned width, unsigned height, int hotspot_x, int hotspot_y,
+send_cursor(int streamfd, unsigned width, unsigned height, int hotspot_x, int hotspot_y,
std::function<void(uint32_t *)> fill_cursor)
{
if (width >= STREAM_MSG_CURSOR_SET_MAX_WIDTH ||
@@ -288,7 +312,7 @@ send_cursor(unsigned width, unsigned height, int hotspot_x, int hotspot_y,
write_all(streamfd, msg.get(), cursor_size);
}
-static void cursor_changes(Display *display, int event_base)
+static void cursor_changes(int streamfd, Display *display, int event_base)
{
unsigned long last_serial = 0;
@@ -310,25 +334,20 @@ static void cursor_changes(Display *display, int event_base)
for (unsigned i = 0; i < cursor->width * cursor->height; ++i)
pixels[i] = cursor->pixels[i];
};
- send_cursor(cursor->width, cursor->height, cursor->xhot, cursor->yhot, fill_cursor);
+ send_cursor(streamfd,
+ cursor->width, cursor->height, cursor->xhot, cursor->yhot, fill_cursor);
}
}
static void
-do_capture(const char *streamport, FILE *f_log)
+do_capture(int streamfd, const char *streamport, FILE *f_log)
{
- streamfd = open(streamport, O_RDWR);
- if (streamfd < 0)
- throw std::runtime_error("failed to open the streaming device (" +
- std::string(streamport) + "): "
- + strerror(errno));
-
unsigned int frame_count = 0;
while (!quit_requested) {
while (!quit_requested && !streaming_requested) {
- if (read_command(true) < 0) {
+ if (read_command(streamfd, true) < 0) {
syslog(LOG_ERR, "FAILED to read command\n");
- goto done;
+ return;
}
}
@@ -370,7 +389,7 @@ do_capture(const char *streamport, FILE *f_log)
syslog(LOG_DEBUG, "wXh %uX%u codec=%u\n", width, height, codec);
- if (spice_stream_send_format(width, height, codec) == EXIT_FAILURE)
+ if (spice_stream_send_format(streamfd, width, height, codec) == EXIT_FAILURE)
throw std::runtime_error("FAILED to send format message");
}
if (f_log) {
@@ -382,23 +401,18 @@ do_capture(const char *streamport, FILE *f_log)
hexdump(frame.buffer, frame.buffer_size, f_log);
}
}
- if (spice_stream_send_frame(frame.buffer, frame.buffer_size) == EXIT_FAILURE) {
+ if (spice_stream_send_frame(streamfd,
+ frame.buffer, frame.buffer_size) == EXIT_FAILURE) {
syslog(LOG_ERR, "FAILED to send a frame\n");
break;
}
//usleep(1);
- if (read_command(false) < 0) {
+ if (read_command(streamfd, false) < 0) {
syslog(LOG_ERR, "FAILED to read command\n");
- goto done;
+ return;
}
}
}
-
-done:
- if (streamfd >= 0) {
- close(streamfd);
- streamfd = -1;
- }
}
#define arg_error(...) syslog(LOG_ERR, ## __VA_ARGS__);
@@ -481,12 +495,13 @@ int main(int argc, char* argv[])
Window rootwindow = DefaultRootWindow(display);
XFixesSelectCursorInput(display, rootwindow, XFixesDisplayCursorNotifyMask);
- std::thread cursor_th(cursor_changes, display, event_base);
+ Stream streamfd(streamport);
+ std::thread cursor_th(cursor_changes, (int) streamfd, display, event_base);
cursor_th.detach();
int ret = EXIT_SUCCESS;
try {
- do_capture(streamport, f_log);
+ do_capture(streamfd, streamport, f_log);
}
catch (std::runtime_error &err) {
syslog(LOG_ERR, "%s\n", err.what());
--
2.13.5 (Apple Git-94)
More information about the Spice-devel
mailing list