This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.

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


Thanks,
Corinna

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

Attachment: signature.asc
Description: PGP signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]