>
> 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()
> ---
> vdagent/vdagent.cpp | 39 ++++++++++++++++++++++++++++++---------
> 1 file changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/vdagent/vdagent.cpp b/vdagent/vdagent.cpp
> index 959881d..d3cbd5f 100644
> --- a/vdagent/vdagent.cpp
> +++ b/vdagent/vdagent.cpp
> @@ -478,12 +478,27 @@ void VDAgent::input_desktop_message_loop()
> CloseDesktop(hdesk);
> }
>
> +#define G_N_ELEMENTS(arr) (sizeof (arr) / sizeof ((arr)[0]))
> +
Oh.. you can use SPICE_N_ELEMENTS, should be available
> 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;
> + DWORD action;
action should be of enumerator type ...
> MSG msg;
> + enum {
> + CONTROL_ACTION,
> + STOP_ACTION,
> + } actions[G_N_ELEMENTS(events)];
... I would put here as
} actions[G_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 +507,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 (%lu)", __func__, action);
> _running = false;
> }
> }
Beside these small changes looks fine.
Acked-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel