Re: About the dll search algorithm of dlopen (patch-r3)

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

Hi Corinna,

On 08/26/2016 01:18 PM, Corinna Vinschen wrote:
> On Aug 26 12:59, Corinna Vinschen wrote:
>> On Aug 25 19:48, Michael Haubenwallner wrote:
>>> Using tmp_pathbuf now, wrapped behind some trivial allocator - which
>>> might fit better somewhere else than to dlfcn.cc?
>>>
>>> BTW: Is it really intended for tmp_pathbuf to have a single active
>>> instance (per thread) at a time?
>>
>> Well, yes.  tmp_pathbuf is meant to be initialized on function entry
>> (more or less, depends).  It's supposed to exist only once per frame.
>> When the frame goes out of scope, the tmp_pathbuf usage counter is
>> restored to the values of the parent frame.
>>
>>> +   ATTENTION: Requesting memory from an instance of tmp_pathbuf breaks
>>> +   when another instance on a newer stack frame has provided memory. */
>>
>> I don't understand this comment, though.  tmp_pathbuf can be used
>> multiple times in the same thread stackm see other Cygwin functions.
>> What you can't do is to call a function, instantiate tmp_pathbuf,
>> allocate memory in the called function, and then use it in the caller.
>> Well, you *can* do that, but if you do this more than once, the same
>> memory region is reused.
>> That's the whole idea of tmp_pathbuf.
>> Temporary per-thread memory for the current frame and it's child frames
>> is allocated.  If a deeper frame using tmp_pathbuf goes out of scope,
>> the memory is not free'd, but recycled next time temporary memory is
>> needed.  The buffers are either 32K or 64K to matches the maximum long
>> path length.  They are now used for any purpose where larger temporary
>> per-thread memory is needed, but providing temporary long path buffers
>> without killing the stack was their original purposes.
> 
> So I also don't quite understand splitting off tmp_pathbuf_allocator.

Sorry, 've expected to have provided enough reasoning here:
https://cygwin.com/ml/cygwin-developers/2016-08/msg00016.html

Actually it is vstrlist doing the alloc+free, and I do expect vstrlist
to be useful even in scenarios where it does not work to ignore it's
free calls, but requires the allocator to fully maintain the alloc'ed
regions to allow for free'd regions to become reused.
And as I've stumbled over 3 different memory providers already (malloc,
cmalloc, tmp_pathbuf), I don't think vstrlist should know anything
about tmp_pathbuf at all - otherwise it should be named tmp_vstrlist.

Same stands for pathfinder, which needs the allocator interface reference
only for construction of the embedded vstrlist that holds the searchdirs.

> Why didn't you just include a tmp_pathbuf member into class pathfinder,
> kind of like this:
> 
>   /* pathfinder.h */
>   class pathfinder
>   {
>     tmp_pathbuf tp;
>     [...]
>   };
> 
>   /* dlfcn.cc */
>   get_full_path_of_dll()
>   {
>     pathfinder finder ();	/* Has a tmp_pathbuf member */
> 
>     [...]
>     finder.add_basename (basename);
>     [...]
>     finder.add_searchdir (dir, ...);
> 
> 
>     return true;		/* finder goes out of scope, so
> 				   its tmp_pathbuf goes out of scope
> 				   so tmp_pathbuf::~tmp_pathbuf is
> 				   called and all is well. */
>   }

Well, as one stackframe better uses one _single_ instance of tmp_pathbuf,
I prefer to be explicit about this and have pathfinder+vstrlist take a
tmp_pathbuf _reference_:
A member instance would be hidden to some degree, and thus will ask for
trouble in the long run, as it would not be obvious to developers in the
future that this stackframe already _does_ have that single instance and
one should not add (another) one.

In hope to provide graspable thoughts,
/haubi/