<joerg-cyril.hoe...@t-systems.com> writes:
> Hi,
>
>>+ switch (wmm->dwStatus) {
>>+ case MCI_MODE_PLAY:
>>+ case MCI_MODE_PAUSE:
>>+ wmm->dwStatus = MCI_MODE_STOP;
>>+ }
>
>>This is weird. You're "missing" a break,
> You're right, at least a /* fall through */ or break is custom.
>
>>and it seems like an if statement would be more appropriate here, anyway.
> I wouldn't like it. dwStatus is to be considered like a volatile.
> The Switch statements embeds the idea that the value is loaded once,
> then dispatched upon. The code does not depend on that (nor is the
> C compiler prevented from reloading from memory without volatile declaration),
> but that's my model. A chain of If statements would not carry that idea.
>
>
> That's why, in patch 20/25, I wrote:
> + if (wmm->dwStatus == MCI_MODE_STOP || wmm->dwStatus ==
> MCI_MODE_NOT_READY)
> + break;
> + else if (wmm->dwStatus != MCI_MODE_PLAY)
> + continue;
>
> From a pure logic point of view, it could have been more directly expressed
> as:
> else if (wmm->dwStatus != MCI_MODE_PLAY) break;
> or
> if (wmm->dwStatus == MCI_MODE_PAUSE) continue;
> if (wmm->dwStatus != MCI_MODE_PLAY) break;
> but that's not the same. A concurrent thread might
> change dwStatus to PAUSE right between the 2 lines.
If the code depends on that sort of thing then it's broken. If another
thread can change dwStatus you need a critical section or an interlocked
function.
--
Alexandre Julliard
julli...@winehq.org
