[PATCH] Fix memory leak and add small optiomization

[Marco Diego Aurélio Mesquita]

Hi devs!

I investigated this problem a bit more.

It looks like the new approach messes up the way path_positions are
calculated. I think I can soon come up with a solution.

On Wed, Aug 24, 2016 at 11:23 PM, Marco Diego Aurélio Mesquita
<marcodiegomesquita at gmail.com> wrote:
> Hi Dan!
>
> Attached is a patch implementing the optimization the way you
> described. For some reason, check-sort-order fails after this change.
>
> Any tips on how to fix it?
>
>
> On Wed, Aug 24, 2016 at 4:26 PM, Dan Nicholson <nicholson at endlessm.com> wrote:
>> On Mon, Aug 22, 2016 at 6:30 PM, Marco Diego Aurélio Mesquita
>> <marcodiegomesquita at gmail.com> wrote:
>>> Hi Dan,
>>>
>>> Thanks for reviewing my patch. Some points:
>>>
>>> On Mon, Aug 22, 2016 at 7:06 PM, Dan Nicholson <nicholson at endlessm.com> wrote:
>>>> 1. I'd prefer to keep the searching done the way it is where the
>>>> packages hash table is searched rather than open coding the checks in
>>>> has_required_packages. That duplicates too much of the logic in a way
>>>> that's likely to break. In that sense, I'd prefer to skip the
>>>> optimization where package_init isn't run immediately.
>>>>
>>>
>>> I'd really like to test for the presence of each requested .pc file
>>> before calling package_init. Is there a cleaner way to do it?
>>
>> OK, so thinking on it some more, here's my take.
>>
>> Doing the small optimization you have here to check the requested
>> packages before doing the scanning doesn't seem like a big win since
>> in the common case (you have the package) you end up doing the
>> scanning anyway.
>>
>> What would be a good optimization would be to change pkg-config so
>> that it only does the scanning in the --list-all case. In other words,
>> add packages to the hash table as needed instead of always filling it
>> immediately. Here's a strawman approach to that.
>>
>> 1. Factor out the code from scan_dir that actually adds a package to
>> the locations hashtable based on finding a .pc file. Something like
>> add_package(path).
>>
>> 2. Change internal_get_package so that if location is NULL, search for
>> the pc file like scan_dir does and add it to the locations hashtable.
>> If it's still not found, return NULL like it does now.
>>
>> 3. Remove the scan_dir call in package_init. Add a helper function to
>> call scan_dir and use that in main when --list-all is requested.
>>
>> There's some tricky code with scanned_dir_count in scan_dir that would
>> need to continue working in the case where scan_dir was not called.
>> The path_positions hash table needs to keep working correctly.
>>
>>>> 2. The leak can be fixed by simply calling g_list_free at the end of
>>>> process_package_args, right?
>>>>
>>>
>>> AFAICS yes.
>>
>> I would split this out to a separate patch if you still want to fix that.
>>
>> --
>> Dan