[pacman-dev] BUG and PATCH: 'handle' not null after alpm_release()

[Dan McGee]

On Tue, Jan 19, 2010 at 7:09 AM, Markus Meissner <markus at evigo.net> wrote:
> On 01/19/2010 01:34 PM, Xavier Chantry wrote:
>> On Tue, Jan 19, 2010 at 12:37 PM, Markus Meissner <markus at evigo.net> wrote:
>>> 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.
>>>
>>> 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.

I'm with Xavier on this, patch is below. We only call this in one
place, and could really just inline the code in there if we want to do
it right...

And on the "do what it's name indicates" topic, realize that in C
free() doesn't set the passed pointer to NULL; you are expected to
know that value is bogus. We just didn't mark it as such on the return
from our own call to a free()-like function.

>>> 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?)

This is a failure on my part; hard to blame others here. I've thought
about getting it to a release state as otherwise good changes do sit
around unreleased.

The reason we don't do the "third party library friendly" stuff?
Because we get next to zero traffic on the mailing list from you guys.
Seriously, this is the first message from someone in quite some time,
and all the time we pose questions to the ML asking about a API change
and get zero responses.

-Dan


diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c
index 6ab6516..8250256 100644
--- a/lib/libalpm/alpm.c
+++ b/lib/libalpm/alpm.c
@@ -71,6 +71,7 @@ int SYMEXPORT alpm_release(void)
        }

        _alpm_handle_free(handle);
+       handle = NULL;

        return(0);
 }