[PATCH 1/4] dlopen: switch to new pathfinder class

[Michael Haubenwallner]

On 09/01/2016 04:03 PM, Corinna Vinschen wrote:
> On Sep  1 13:05, Michael Haubenwallner wrote:
>> On 08/31/2016 09:12 PM, Corinna Vinschen wrote:
>>> On Aug 31 20:07, Michael Haubenwallner wrote:
>>>> Instead of find_exec, without changing behaviour use new pathfinder
>>>> class with new allocator_interface around tmp_pathbuf and new vstrlist
>>>> class.
>>>> * pathfinder.h (pathfinder): New file.
>>>> * vstrlist.h (allocator_interface, allocated_type, vstrlist): New file.
>>>> * dlfcn.cc (dlopen): Avoid redundant GetModuleHandleExW with RTLD_NOLOAD
>>>> and RTLD_NODELETE.  Switch to new pathfinder class, using
>>>> (tmp_pathbuf_allocator): New class.
>>>> (get_full_path_of_dll): Drop.
>>>> [...]
>>>
>>> Just one nit here:
>>>
>>>> +/* Dumb allocator using memory from tmp_pathbuf.w_get ().
>>>> +
>>>> +   Does not reuse free'd memory areas.  Instead, memory
>>>> +   is released when the tmp_pathbuf goes out of scope.
>>>> +
>>>> +   ATTENTION: Requesting memory from an instance of tmp_pathbuf breaks
>>>> +   when another instance on a newer stack frame has provided memory. */
>>>> +class tmp_pathbuf_allocator
>>>> +  : public allocator_interface
>>>
>>> You didn't reply to
>>> https://cygwin.com/ml/cygwin-developers/2016-08/msg00013.html
>>> So, again, why didn't you simply integrate a tmp_pathbuf member into the
>>> pathfinder class, rather than having to create some additional allocator
>>> class?  I'm probably not the most diligent C++ hacker, but to me this
>>> additional allocator is a bit confusing.
>>
>> Sorry, seems I've failed to fully grasp your concerns firsthand in
>> https://cygwin.com/ml/cygwin-developers/2016-08/msg00016.html
>> Second try to answer:
>> https://cygwin.com/ml/cygwin-developers/2016-09/msg00000.html
> 
> Ok, I see what you mean, but it doesn't make me really happy.
> 
> I'm willing to take it for now but I'd rather see basenames being a
> member of pathfinder right from the start, so you just instantiate
> finder and call methods on it.

The idea to build the basenames list before constructing pathfinder
is that members of the searchdirs list reserve space for the maxlen
of basenames:  This implies that the basenames list must not change
after the first searchdir was registered.

To make sure this doesn't happen I prefer to not provide such an API
at all, rather than to check within some pathfinder::add_basename ()
method and abort if there is some searchdir registered already.

> Given that basenames is a member,
> you can do the allocator stuff completely inside the pathfinder class.

Moving the allocator into pathfinder would work then, but still the
tmp_pathbuf instance to use has to be provided as reference.

/haubi/