On Sun, 2011-11-27 at 06:22 +0100, David Henningsson wrote: > On 11/26/2011 04:08 PM, Tanu Kaskinen wrote: > > with the following tweaks: > > > > - Added an assertion to pa_device_port_hashmap_free(). > > For the record, I just want to say that I did not change this not > because I chose to ignore your comment, but because I think this is > error prone behaviour for destructors, and I believe Colin agrees with > me [1]. But maybe the consistency argument weighs heavier. Yes, I made the change mainly for consistency. Currently when I see any pa_*_free() function called in the code, I know what it does, and I want to keep it that way (and I don't want to have to remember exceptions beyond pa_xfree). I might not necessarily oppose a patch that changes all free functions to be no-op on NULL. > And actually, > there is no "if (c->ports)" in pa_card_free, is this intentional? Yes. The check is not necessary. On the other hand, whether or not it's necessary depends on the details of card initialization, which might change at some point, so having the check there would have been more future proof. I think such change is unlikely, which is why I didn't add the check, but it was very much a 50/50 decision. > > - Fixed a compilation issue in alsa-mixer.c. When the pa_core parameter > > was added to pa_device_port_new(), the calling code was not updated. > > Aha, maybe this was fixed up in patch 6/6 (I did test that my code > compiled before I submitted it). Yep. -- Tanu
