Joerg-Cyril.Hoehle@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.