February 2012 - Wine-Devel - list.winehq.org

Re: [PATCH] winealsa: snd_card_get_name uses strdup (Valgrind).

by Christian Costa

In that case you don't need the if (!err) condition to free the buffer otherwise you leak the string return by strdup. Christian 2012/2/1 Lauri Kenttä <lauri.kentta(a)gmail.com> > --- > dlls/winealsa.drv/mmdevdrv.c | 10 +++++++--- > 1 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c > index 0bedcc5..e434038 100644 > --- a/dlls/winealsa.drv/mmdevdrv.c > +++ b/dlls/winealsa.drv/mmdevdrv.c > @@ -363,7 +363,7 @@ static HRESULT alsa_enum_devices(EDataFlow flow, WCHAR > **ids, char **keys, > for(err = snd_card_next(&card); card != -1 && err >= 0; > err = snd_card_next(&card)){ > char cardpath[64]; > - const char *cardname; > + char *cardname; > WCHAR *cardnameW; > snd_ctl_t *ctl; > DWORD len; > @@ -376,16 +376,18 @@ static HRESULT alsa_enum_devices(EDataFlow flow, > WCHAR **ids, char **keys, > continue; > } > > - if((err = snd_card_get_name(card, (char **)&cardname)) < 0){ > + if((err = snd_card_get_name(card, &cardname)) < 0){ > WARN("Unable to get card name for ALSA device %s: %d (%s)\n", > cardpath, err, snd_strerror(err)); > /* FIXME: Should be localized */ > - cardname = "Unknown soundcard"; > + cardname = strdup("Unknown soundcard"); > } > > len = MultiByteToWideChar(CP_UNIXCP, 0, cardname, -1, NULL, 0); > cardnameW = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR)); > if(!cardnameW){ > + if(!err) > + free(cardname); > snd_ctl_close(ctl); > return E_OUTOFMEMORY; > } > @@ -394,6 +396,8 @@ static HRESULT alsa_enum_devices(EDataFlow flow, WCHAR > **ids, char **keys, > alsa_get_card_devices(stream, ids, keys, num, ctl, card, > cardnameW); > > HeapFree(GetProcessHeap(), 0, cardnameW); > + if(!err) > + free(cardname); > > snd_ctl_close(ctl); > } > -- > 1.7.9 > > > >

Feb. 1, 2012

Re: snd_card_get_name uses strdup (Valgrind).

by Michael Stefaniuc

Hello Joerg, On Wed, Feb 01, 2012 at 01:37:44PM +0100, Joerg-Cyril.Hoehle(a)t-systems.com wrote: > there's a small memory leak in winealsa, tentatively plugged by > patches #83353, #83334 and #82900. > > It would be nice if somebody who feels like struggling with const warnings in gcc > would get a fix accepted in git. > > The dilemma: a literal "foo" is a const char*, free() does not want a const > and casts are the root of all evil in C. something like the attached patch should be more palatable. bye michael

Feb. 1, 2012

snd_card_get_name uses strdup (Valgrind).

by Joerg-Cyril.Hoehle＠t-systems.com

Hi there's a small memory leak in winealsa, tentatively plugged by patches #83353, #83334 and #82900. It would be nice if somebody who feels like struggling with const warnings in gcc would get a fix accepted in git. The dilemma: a literal "foo" is a const char*, free() does not want a const and casts are the root of all evil in C. Thank you, Jörg Höhle

Feb. 1, 2012

Re: [PATCH 2/3] winecoreaudio: Implement the lock-free callback design.

by Joerg-Cyril.Hoehle＠t-systems.com

Hi, > What does that not guarantee (that you'd like to have)? Consider this scenario: Play: EnterCS-2 do some stuff, start playing state=Playing; LeaveCS-2 Stop: EnterCS-1 do stuff LeaveCS-1 ; because we want to wait for something to finish wait EnterCS-3 state=Stopped LeaveCS-3 Now issue Stop + Play from 2 threads. Play will start playing while the Stop thread is waiting. Then CS-3 will stomp over the state, leading to inconsistent data structures. The player thread is still running while the state variable indicates Stopped. What can be done? A. Design the system such that waiting can happen inside the CS. winecoreaudio now can do that. B. Design the system such that it accomodates transition states. This means a much more complex state machine than the API describes. "Can I restart playing if I'm mid-closing? mid-stopping?" "Am I allowed to refuse starting playing when I acknowledged Stop earlier such that the app now believes I've stopped and submits the next song?" It also likely begs for InterlockedCompareExchange and the sort. You then have to be careful about which slots are valid when. It is hard to get correct. (An example of using Interlocked* is in dlls/mci*/mci*.c notification, can you spot the one fault?) OO programmers (should) know this well: while mid-method, your object state is not consistent; calling another method may call out to other objects which can again call one of your API methods. But the average API methods are not preprared to face the temporary inconsistent state. Erlang avoids this by design. C. Restart the transaction Add a loop atop CS-1 After exiting CS-1 and waiting, check whether the state is the one you wish to reach. Otherwise restart the loop. (An example is PlaySound in dlls/winmm/playsound.c). All implementations of interlocked lists do something like this: check the final state and restart if not satisfying. A variation on the above scenario, found in the MCI: the player thread wants to enter Stopped state at the end, after waiting for all resources to be returned. When is it allowed to modify the state variable? Regards, Jörg Höhle

Feb. 1, 2012

Re: Updated clang-static-analyzer results (just in time for 1.4 ;))

by Austin English

On Tue, Jan 31, 2012 at 08:19, Jacek Caban <jacek(a)codeweavers.com> wrote: > Hi Austin, > > On 01/31/12 06:11, Austin English wrote: >> Enjoy: http://www.mediafire.com/?aiomfk1cr9bc61y >> Using: >> wine-1.4-rc1-34-g01947e0 >> >> LLVM commit 683d1e1d449e6e790ff4c0b6340a4bbf144ff919 / git-svn-id: >> http://llvm.org/svn/llvm-project/llvm/trunk(a)149340 >> 91177308-0d34-0410-b5e6-96231b3b80d8 >> >> austin(a)aw21 ~ $ sha1sum scan-build-2012-01-30-1.tar.bz2 >> 8b3b2573c3595c805431b2ab40c6958805f37497 scan-build-2012-01-30-1.tar.bz2 > > This produces tons of cases in tests that we don't care about. I'd > suggest running such analyzes with --disable-tests. Here you go: http://www.mediafire.com/?69ce12mfycgfa3v b9fa3effac61568af7975ef304f3342e2c3d8ba5 scan-build-2012-02-01-1.tar.bz2 -- -Austin

Feb. 1, 2012

Re: [PATCH 2/3] winecoreaudio: Implement the lock-free callback design.

by Peter Rosin

Joerg-Cyril.Hoehle(a)t-systems.com skrev 2012-01-31 15:23: > Peter Rosin wrote: >>> Of the same vein, one liners like >>> EnterCS >>> This-> playing = StateStopped; >>> LeaveCS >>> are suspicious too. What does that guarantee? >> Perhaps that another thread doing as below isn't fooled into >> calling both one() and three()? > Right. The next question is: > What does that not guarantee (that you'd like to have)? It does not guarantee that I have money in my pocket. Seriously, I'm not sure in what way I'm supposed to not get what a CS does or doesn't do. Please spell it out! Cheers, Peter

Feb. 1, 2012