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

[Markus Meissner]

On 01/19/2010 03:01 PM, Dan McGee wrote:
> 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.

Alright here - Actually I have to admit that just one line is always
better, than changing function prototypes and stuff^^

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

I didn't want to blame someone, and that's why I asked if I maybe missed
it, because I just finished my graduation last month there was absolutly
not enough time to even read the ML completly. But hopefully this will
change, I just pushing the lib to work with recent git and I have to
think later what to do with the stable release once this is done.

Don't get me wrong. I know that all these "tplf"
(third-party-library-friendly) wishes produce a not so small extra
amount of work on your side and if there is no feedback from us
tplf-guys I can absolutly understand that the priority for such things
is low. The only thing I can say right now is that I'll try to read the
ML regularly and as all those changes are discussed here, this is more
than enough right now.

Greets
Markus

> 
> 
> 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);
>  }
> 
> 
>