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

[Michael Haubenwallner <michael dot haubenwallner at ssi-schaefer dot com>]

On 08/31/2016 09:12 PM, Corinna Vinschen wrote:
> Hi Michael,
> 
> 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

> The rest of the patch looks good.  I'll look further into the patchset
> later tomorrow.

Thanks!
/haubi/