dsound: Don't bother shrinking the secondary buffer list.
mstefani at redhat.com
Thu Sep 27 04:00:37 CDT 2012
as always it depends.
On 09/27/2012 09:37 AM, Joerg-Cyril.Hoehle at t-systems.com wrote:
>> +#include <assert.h>
> your patch is already committed, yet I believe that the least that dsound needs is new asserts.
> Asserts in dsound have been a PITA in Wine for the last decade.
> It's ok for the kernel to crash in an assert. IMHO it is not ok for an optional thing like sound.
> The assert's backtrace will not help anybody, because the logical error will have happened much earlier.
I disagree. It helps GyB to open regression bug reports and it helps me
to fix them.
> Instead, a plain ERR would have been just as telling. Let's quote
> "ERR: Messages in this class indicate serious errors in Wine,
> such as as conditions that should never happen by design."
> I recommend:
> - assert(device->buffers == pDSB);
> + if (pdevice->buffers != pDSB)
> + ERR("broken device buffer %p\n", device->buffers);
> HeapFree(GetProcessHeap(), 0, device->buffers);
> device->buffers = NULL;
> The ERR is IMHO revealing enough in a user's log.
That's too late if it hits the user.
I had reviewed the code: That code is an implementation detail
completely outside the control of the applications. Barring a memory
corruption there is no way for it to fail. The assert is there for two
- To crash during "make test" when the next developer touches that code.
- As a comment less likely to be removed by Alexandre ;)
> If a sound component detects a bogus state,
> I'd very much prefer that it disables sound -- without crashing.
In general "it depends" of course.
In this particular case the ERR would be useless noise as the assert is
only for the last sound buffer that is removed; usually at program exit.
Until then the code does what you prefer: silently drops sound buffers
without crashing thus deferring the crash to the program exit. A crash
on program exit does get reported, more so than an ERR on the command line.
More information about the wine-devel