[PATCH 0/1] MR723: winepulse.drv: Check state after connect

[PATCH 1/1] winepulse.drv: Check state after connect

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

---
 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));

[Huw Davies (＠huw)]

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?

[Jacek Caban (＠jacek)]

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.

[Huw Davies (＠huw)]

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:
```c
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;
```

[Alistair Leslie-Hughes (＠alesliehughes)]

@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.