<Joerg-Cyril.Hoehle(a)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 julliard(a)winehq.org