> > As _stop_event is not mandatory, we are initializing the events array > with something that might be NULL. The problem is with the following > patch as I'm introducing a new non mandatory event and logic starts to > get a bit hard to follow. > > So this patch makes explicit if we are setting or receiving a > _stop_event or not and uses an auxiliary enum and array 'actions' to > work around this non mandatory events and the return value of > MsgWaitForMultipleObjectsEx() > > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx> > Acked-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > --- > > Suggestions taken from Frediano; > The diff from previous patch: > https://paste.fedoraproject.org/406316/47093493/ > > --- > vdagent/vdagent.cpp | 39 ++++++++++++++++++++++++++++----------- > 1 file changed, 28 insertions(+), 11 deletions(-) > > diff --git a/vdagent/vdagent.cpp b/vdagent/vdagent.cpp > index c67f30c..8e7ba2b 100644 > --- a/vdagent/vdagent.cpp > +++ b/vdagent/vdagent.cpp > @@ -20,6 +20,7 @@ > #include "display_setting.h" > #include "file_xfer.h" > #include "ximage.h" > +#include "spice-protocol/spice/macros.h" Looking at Makefile.am and other sources this should work and be more consistent: #include <spice/macros.h> > #undef max > #undef min > #include <wtsapi32.h> > @@ -480,10 +481,22 @@ void VDAgent::input_desktop_message_loop() > > void VDAgent::event_dispatcher(DWORD timeout, DWORD wake_mask) > { > - HANDLE events[] = {_control_event, _stop_event}; > - DWORD event_count = _stop_event ? 2 : 1; > + HANDLE events[2]; > + DWORD event_count = 1; > DWORD wait_ret; > MSG msg; > + enum { > + CONTROL_ACTION, > + STOP_ACTION, > + } actions[SPICE_N_ELEMENTS(events)], action; > + > + events[0] = _control_event; > + actions[0] = CONTROL_ACTION; > + if (_stop_event) { > + events[event_count] = _stop_event; > + actions[event_count] = STOP_ACTION; > + event_count++; > + } > > wait_ret = MsgWaitForMultipleObjectsEx(event_count, events, timeout, > wake_mask, MWMO_ALERTABLE); > if (wait_ret == WAIT_OBJECT_0 + event_count) { > @@ -492,19 +505,25 @@ void VDAgent::event_dispatcher(DWORD timeout, DWORD > wake_mask) > DispatchMessage(&msg); > } > return; > + } else if (wait_ret == WAIT_IO_COMPLETION || wait_ret == WAIT_TIMEOUT) { > + return; > + } else if (wait_ret < WAIT_OBJECT_0 || wait_ret > WAIT_OBJECT_0 + > event_count) { > + vd_printf("MsgWaitForMultipleObjectsEx failed: %lu %lu", wait_ret, > GetLastError()); > + _running = false; > + return; > } > - switch (wait_ret) { > - case WAIT_OBJECT_0: > + > + action = actions[wait_ret - WAIT_OBJECT_0]; > + switch (action) { > + case CONTROL_ACTION: > handle_control_event(); > break; > - case WAIT_OBJECT_0 + 1: > + case STOP_ACTION: > + vd_printf("%s: received stop event", __func__); > _running = false; > break; > - case WAIT_IO_COMPLETION: > - case WAIT_TIMEOUT: > - break; > default: > - vd_printf("MsgWaitForMultipleObjectsEx failed: %lu %lu", wait_ret, > GetLastError()); > + vd_printf("%s: action not handled (%d)", __func__, action); > _running = false; > } > } > @@ -906,8 +925,6 @@ bool VDAgent::handle_max_clipboard(VDAgentMaxClipboard > *msg, uint32_t size) > return true; > } > > -#define MIN(a, b) ((a) > (b) ? (b) : (a)) > - > bool VDAgent::write_clipboard(VDAgentMessage* msg, uint32_t size) > { > uint32_t pos = 0; Beside that patch is fine. Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel