[PATCH 0/1] MR723: winepulse.drv: Check state after connect
From: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> --- dlls/winepulse.drv/pulse.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 463f4c28d00..1878c367b97 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -391,6 +391,9 @@ static HRESULT pulse_connect(const char *name) break; } + if (pa_context_get_state(pulse_ctx) != PA_CONTEXT_READY) + goto fail; + TRACE("Connected to server %s with protocol version: %i.\n", pa_context_get_server(pulse_ctx), pa_context_get_server_protocol_version(pulse_ctx)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/723
Huw Davies (@huw) commented about dlls/winepulse.drv/pulse.c:
break; }
+ if (pa_context_get_state(pulse_ctx) != PA_CONTEXT_READY) + goto fail; +
This whole loop doesn't look right. @jacek do happen to know how this is supposed to work? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/723#note_7204
On Tue Aug 30 09:06:06 2022 +0000, Huw Davies wrote:
This whole loop doesn't look right. @jacek do happen to know how this is supposed to work? Yeah, it seems that pulse_cond_wait return value handling is reversed. I think that 6f632affa75a meant to handle it similarly to equivalent pulse_test_connect.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/723#note_7205
On Tue Aug 30 09:36:33 2022 +0000, Jacek Caban wrote:
Yeah, it seems that pulse_cond_wait return value handling is reversed. I think that 6f632affa75a meant to handle it similarly to equivalent pulse_test_connect. Even then one would typically check the predicate first, e.g. something like:
while ((state = pa_context_get_state(pulse_ctx) != PA_CONTEXT_READY) &&
state != PA_CONTEXT_FAILED && state != PA_CONTEXT_TERMINATED)
pulse_cond_wait();
if (state != PA_CONTEXT_READY) goto fail;
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/723#note_7212
@jacek Yes, your right the logic is inverted. @huw I'll update the patch to the code above, it seem more consistent with other parts of the code. Thank you both for the review. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/723#note_7276
participants (4)
-
Alistair Leslie-Hughes -
Alistair Leslie-Hughes (＠alesliehughes)
-
Huw Davies (＠huw)
-
Jacek Caban (＠jacek)