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

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

On Sep  1 13:05, Michael Haubenwallner wrote:
> 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

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.  Given that basenames is a member,
you can do the allocator stuff completely inside the pathfinder class.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Attachment: signature.asc
Description: PGP signature