sound issues (which are not cared since 1.3.1)

Hi guys,

there are bug reports showing random crashes with audio on, during
winecfg etc. I was bit by those too... There is probably something
wrong how regressions are handled here, which goes on and thus there
are unneeded bugreports which cost everyone's time to read,
evaluate, search, mark, retest... Why don't you use revert more
offen like kernel guys?


Back to the sound issue - my zero programming skill standing just on
sound despair, i guess in dlls/winealsa.drv/mixer.c ALSA_MixerInit()
should be

int card, err, capcontrols=0

otherwise things like:

mixdev[mixnum].chans += capcontrols;

mixdev[mixnum].lines = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
sizeof(line) * mixdev[mixnum].chans);

mixdev[mixnum].controls = HeapAlloc(GetProcessHeap(),
HEAP_ZERO_MEMORY, sizeof(control) *
CONTROLSPERLINE*mixdev[mixnum].chans);


goes crazy like in http://bugs.winehq.org/show_bug.cgi?id=25231#c2.
Could someone review this piece of code, please?


Thanks,
W.

[Marcus Meissner]

On Sat, Dec 25, 2010 at 11:22:48PM +0100, wylda(a)volny.cz wrote:
> Hi guys,
>
> there are bug reports showing random crashes with audio on, during
> winecfg etc. I was bit by those too... There is probably something
> wrong how regressions are handled here, which goes on and thus there
> are unneeded bugreports which cost everyone's time to read,
> evaluate, search, mark, retest... Why don't you use revert more
> offen like kernel guys?
>
> Back to the sound issue - my zero programming skill standing just on
> sound despair, i guess in dlls/winealsa.drv/mixer.c ALSA_MixerInit()
> should be
>
> int card, err, capcontrols=0
>
> otherwise things like:
>
> mixdev[mixnum].chans += capcontrols;
>
> mixdev[mixnum].lines = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
> sizeof(line) * mixdev[mixnum].chans);
>
> mixdev[mixnum].controls = HeapAlloc(GetProcessHeap(),
> HEAP_ZERO_MEMORY, sizeof(control) *
> CONTROLSPERLINE*mixdev[mixnum].chans);
>
> goes crazy like in http://bugs.winehq.org/show_bug.cgi?id=25231#c2.
> Could someone review this piece of code, please?

well spotted, the patch incorrectly removed the = 0.

I submitted a patch readding it.

This specific regression I did not hear about it yet, if someone sees
such a regression on bisect, please get the right people in the bugzilla
(like the author of the patch) and/or post to the list pointing out
the regression.

The point is ... speak up if you see such a clear regression.

Ciao, Marcus

> > int card, err, capcontrols=0
> >
> > otherwise things like:
> >
> > mixdev[mixnum].chans += capcontrols;
> >
> > mixdev[mixnum].lines =
> > HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
> > sizeof(line) * mixdev[mixnum].chans);
> >
> > goes crazy like in
> > http://bugs.winehq.org/show_bug.cgi?id=25231#c2.
> well spotted, the patch incorrectly removed the = 0.
>
> I submitted a patch readding it.

Hi again!

Many thanks Marcus for the patch and getting it git-official :)

I wonder if someone could write a Wiki page about different
techniques how beginners like me can spot and easily test for this
kind of wine's code problems.

Thanks to this bug, Wine-1.3.10 + winecfg's audio tab could be taken
as the example and wiki could show how to use different tools
(warn+heap, valgrind, gcc options, ...) and what tools' output to
expect in case of such bugs.


Regards,
W.

PS: Marcus, i noticed "found by my overflow checking patches" in
http://source.winehq.org/patches/data/69925. Are these patches
publicly available? ;)