On 01/19/2010 01:34 PM, Xavier Chantry wrote: > On Tue, Jan 19, 2010 at 12:37 PM, Markus Meissner <[email protected]> wrote: >> Hi pacman dev list, >> >> I am the guy behind pyalpmm and I found a not so small bug in libalpm. >> >> As I wrote some unit-tests for pyalpmm I needed to open and close a >> libalpm-"session" multiple times without restarting the calling >> application. Something like that: >> >> 1. alpm_initialize(); >> 2. /** do some operation here **/ >> 3. alpm_release(); >> 4. alpm_initialize(); >> 5. alpm_release(); >> >> But I couldn't do this, because the alpm_initialize() call in line 4 >> always threw an error (errno: PM_ERR_HANDLE_NOT_NULL), which translates >> to: "library already initialized". This is obviously not true, because >> the alpm_release() function was successfully called. >> >> >> I checked the code and found out that this line threw the error: >> >> ASSERT(handle != NULL, RET_ERR(PM_ERR_HANDLE_NULL, -1)); >> >> What means that the handle is not correctly reseted to NULL, but the >> _alpm_free_handle() function, which is called by alpm_release() has a >> FREE(handle) inside. So here is my explanation why this happens: >> >> The handle pointer is passed to _alpm_free_handle() as a value, so the >> pointer value itself is copied. This works perfect for freeing the >> contents of the handle struct, BUT setting this handle pointer to NULL >> just sets the locally copied pointer to NULL, what means the original >> handle variable remains un-touched. So every alpm_initialize() call >> after the first one will throw this error, no matter if you called >> alpm_release() or not. >> >> So to get finally end, I fixed it and the patch is (against latest git) >> attached to this email. Hopefully my explanation is enough to sketch the >> problem. Solving it, is really easy, just change the _alpm_free_handle() >> function to take a pointer of a pointer of pmhandle_t as an argument, >> change the references to handle inside the function and slightly change >> the _alpm_free_handle() call in alpm_release(). That's how it is done >> with the attached patch... >> > > What about just doing handle = NULL after the _alpm_free_handle() call ?
Yes also thought about that, but I don't like scattering such things around. I mean, _alpm_free_handle() should do what it's called like. So you also don't have to look at different places, if you want to know what gets freed. But honestly I don't mind a lot how, as long as this will be fixed at some point in maybe near future, because it undermines the way I like to write unit tests ;) > >> Finally I also have a question: I saw many changes in the recent git >> compared to the latest release. Is there an estimated release date? >> > > Not afaik. There was a plan to include some features (mainly GPG) for > the next release that no one is working on. > I wonder if we should not release anyway, because there are already > some nice changes. I would absolutly second a release, this would help me a lot as the API changed at some points. It also would be incredibly great, if the API changes would be a little more "third-party-library" friendly. Like first marking items as deprecated for one minor release, before removing them. I think there are some people working with libalpm who would appreciate this, too - thinking about Dario Freddi for example. (Or am I just too inattentive/slow and there was a deprecation-time for things like alpm_trans_addtarget or the ALPM_TRANS_TYPEs?) greets Markus
