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


On Sep  9 10:19, Michael Haubenwallner wrote:
> On 09/08/2016 01:58 PM, Corinna Vinschen wrote:
> > On Sep  2 13:36, Michael Haubenwallner wrote:
> >> On 09/02/2016 10:52 AM, Corinna Vinschen wrote:
> >>> On Sep  2 10:05, Michael Haubenwallner wrote:
> >>>> Moving the allocator into pathfinder would work then, but still the
> >>>> tmp_pathbuf instance to use has to be provided as reference.
> >>>
> >>> Hmm, considering that a function calling your pathfinder *might*
> >>> need a tmp_pathbuf for its own dubious purposes, this makes sense.
> >>> That could be easily handled via the constructor I think:
> >>>
> >>>   tmp_pathbuf tp;
> >>>   pathfinder finder (tp);
> >>>
> >>> Still, since I said I'm willing to take this code as is, do you want me
> >>> to apply it this way for now or do you want to come up with the proposed
> >>> changes first?
> >>
> >> As I do prefer both pathfinder and vstrlist to not know about tmp_pathbuf
> >> in particular but a generic memory provider only: Yes, please apply as is.
> > 
> > Done, minus patch 4.
> 
> Then my original problem with dlopen isn't fixed yet, where some application
> code within /opt/app/bin/app.exe does dlopen(libAPP.dll), currently finding
> /usr/bin/libAPP.dll although app.exe linked against /opt/app/bin/libAPP.dll.

This change has quite a potential to break existing code.  I'm not
confident to do this indiscriminately.  Like Glibc, we can invent
implementation-specific RTLD_xxx flags, though, so why not use some
RTLD_APPDIR or something like that?

> However, thank you anyway!
> 
> > I still think that there should be only a single
> > pathfinder object and the rest encapsulated within and called via some
> > pathfinder member function.  I'll look into it later this year.
> 
> A thought to avoid the extra tmp_pathbuf_allocator class:
> Have cygmalloc.h (or similar) declare the allocator interface,
> to allow for tmp_pathbuf to implement it by itself.
> 
> The complete idea is to have another allocator implementation using
> cmalloc+cfree, as well as one more using malloc+free. Probably there
> is use for another one using VirtualAlloc+VirtualFree as well.
> 
> Then even path_conv might utilize the allocator interface, using the
> cmalloc+cfree implementation (provided by cygmalloc.h) by default...

Why the additional indirection level?  Every allocation type has it's
special properties and IMHO they should be visible to the reader.  As
the writer of the code you have to think about the right allocation
anyway.  A generic allocator hides the details and, in the worst case,
slows down the allocation operation due to additional checks otherwise
only required in bordercases.

What we *really* need desperately is another malloc/free implementation
which is more threading-friendly than the current one.  I made an effort
a few years back trying ptmalloc3 instead of dlmalloc, but encountered
a few problems so I had to revert it.  Still, changing to ptmalloc or
jemalloc would be cool.

Let's continue in November.


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]