This is the mail archive of the cygwin-developers 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: About the dll search algorithm of dlopen (patch-r3)


Hi Corinna,

On 08/22/2016 08:37 PM, Corinna Vinschen wrote:
> Hi Michael,
> 
> On Aug 22 18:02, Michael Haubenwallner wrote:
>> What about this one: 've dropped any templates now,
>> while keeping the basenames logic inside dlopen.
> 
> I was going to reply to your other mail but I'll deferred that for now
> and looked into your code instead.  I think we can basically go along
> with this.  A few points, though.

besides addressing your concerns I've also split the patch one more time:
1) switch dlopen to pathfinder, not changing search behaviour yet
2) fix pathfinder to search basenames within single dir

> 
>> +      /* Finally we better have some fallback. */
>> +      finder.add_searchdir ("/usr/lib", -1);
> 
> You're not adding /usr/bin here because your code contains automatic
> code for lib -> bin duplication... see (*) below.
> 
>> @@ -113,6 +103,7 @@ dlopen (const char *name, int flags)
>>  {
>>    void *ret = NULL;
>>  
>> +  debug_printf ("flags %d for %s", flags, name);
> 
> Stray debug_printf?  Otherwise, if you have a reason to see the
> flags, please provide an extra patch for this.

've been more after the name passed to dlopen, actually.

> 
>> diff --git a/winsup/cygwin/pathfinder.h b/winsup/cygwin/pathfinder.h
>> new file mode 100644
>> index 0000000..3453692
>> --- /dev/null
>> +++ b/winsup/cygwin/pathfinder.h
>> @@ -0,0 +1,112 @@
>> +/* pathfinder.h: find one of multiple file names in path
>> +
>> +   Copyright 2016 Red Hat, Inc.
> 
> Minor point:  Please drop the "Copyright ..., Red Hat" line.  With the
> new LGPL copyright regime we removed them.  They are not required and
> keeping them in shape is awkward.

ok.

> 
>> +      /* Search "x/bin:x/lib" for "x/lib" */
>> +      if (dirlen >=4 && !strncmp (dir + dirlen - 4, "/lib", 4))
>> +	/* prealloc buffer in searchdir for any basename we will search for */
>> +	searchbuffers_.appendv (dir, dirlen - 4, "/bin", 4, "/", 1 + basenames_maxlen_, NULL);
>> +
>> +      /* prealloc buffer in searchdir for any basename we will search for */
>> +      searchbuffers_.appendv (dir, dirlen, "/", 1 + basenames_maxlen_, NULL);
>> +  }
> 
> (*) Yuk!  Do we really, *really* want that?  The redirection from
>     /usr/lib to /usr/bin is only done for system libs, and only because
>     otherwise we had trouble starting Cygwin from CMD or the Explorer
>     GUI "Run..." box.  We can't change this without breaking everything
>     since we *do* depend on the Windows loader yet.
>     
>     However, as long as this is restricted to /usr/lib, /usr/bin, it's a
>     closed system under control of "the distro".  If you extend this to
>     *any* external path ending in "lib", isn't it inherently dangerous
>     to automate this under the hood, without the application's consent?
>     Or, FWIW, the user's consent in case of LD_LIBRARY_PATH?

've split into add_lib_searchdir (), used for "/usr/lib" only.

Should be a separate patch anyway if really necessary.

> 
>> diff --git a/winsup/cygwin/vstrlist.h b/winsup/cygwin/vstrlist.h
>> new file mode 100644
>> index 0000000..ecbdc64
>> --- /dev/null
>> +++ b/winsup/cygwin/vstrlist.h
>> [...]
>> +    void * operator new (size_t class_size, char const * part0, va_list parts)
>> +    {
>> +      char const * part = part0;
>> +      va_list partsdup;
>> +      va_copy (partsdup, parts);
>> +      size_t bufferlength = 0;
>> +      while (part)
>> +	{
>> +	  int partlen = va_arg (partsdup, int);
>> +	  if (partlen < 0)
>> +	    partlen = strlen (part);
>> +	  bufferlength += partlen;
>> +	  part = va_arg (partsdup, char const *);
>> +	}
>> +      va_end (partsdup);
>> +
>> +      return cmalloc_abort (HEAP_STR, class_size + bufferlength);
>> +    }
> 
> Uhm... I don't think this is correct.  Using cmalloc and friends has a
> specific purpose.  Only internal datastructures which are inherited by
> child processes via fork/exec are supposed to be allocated on the
> cygheap.  The cygheap really shouldn't be used freely for other
> purposes.  You could leak it to child processes if another thread forks
> during your thread's call to dlopen.  Also, especially on 32 bit the
> pressure on the cygheap is not insignificant.  Please use malloc/free
> instead.

Accepted - wasn't aware of cygheap's dedication to fork only.

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?

> 
> Other than that your patch looks pretty well to me.  Except, uhm...
> maybe you want to comment the new classes with a usage description?

Done (tried at least).

Thanks!
/haubi/
>From e79e096ad8e2b85f6fb098b4f224db94eb6e406f Mon Sep 17 00:00:00 2001
From: Michael Haubenwallner <michael.haubenwallner@ssi-schaefer.com>
Date: Tue, 31 May 2016 13:09:11 +0000
Subject: [PATCH 1/4] dlopen: switch to new pathfinder class

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 (get_full_path_of_dll): Switch to new pathfinder class, using
(tmp_pathbuf_allocator): New class.
---
 winsup/cygwin/dlfcn.cc     | 139 ++++++++++++-----
 winsup/cygwin/pathfinder.h | 141 +++++++++++++++++
 winsup/cygwin/vstrlist.h   | 371 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 611 insertions(+), 40 deletions(-)
 create mode 100644 winsup/cygwin/pathfinder.h
 create mode 100644 winsup/cygwin/vstrlist.h

diff --git a/winsup/cygwin/dlfcn.cc b/winsup/cygwin/dlfcn.cc
index 255a6d5..5528d35 100644
--- a/winsup/cygwin/dlfcn.cc
+++ b/winsup/cygwin/dlfcn.cc
@@ -20,6 +20,74 @@ details. */
 #include "cygtls.h"
 #include "tls_pbuf.h"
 #include "ntdll.h"
+#include "pathfinder.h"
+
+/* 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
+{
+  tmp_pathbuf & tp_;
+  union
+    {
+      PWCHAR wideptr;
+      void * voidptr;
+      char * byteptr;
+    }    freemem_;
+  size_t freesize_;
+
+  /* allocator_interface */
+  virtual void * alloc (size_t need)
+  {
+    if (need == 0)
+      need = 1; /* GNU-ish */
+    size_t chunksize = NT_MAX_PATH * sizeof (WCHAR);
+    if (need > chunksize)
+      api_fatal ("temporary buffer too small for %d bytes", need);
+    if (need > freesize_)
+      {
+	/* skip remaining, use next chunk */
+	freemem_.wideptr = tp_.w_get ();
+	freesize_ = chunksize;
+      }
+
+    void * ret = freemem_.voidptr;
+
+    /* adjust remaining, aligned at 8 byte boundary */
+    need = need + 7 - (need - 1) % 8;
+    freemem_.byteptr += need;
+    if (need > freesize_)
+      freesize_ = 0;
+    else
+      freesize_ -= need;
+
+    return ret;
+  }
+
+  /* allocator_interface */
+  virtual void free (void *)
+  {
+    /* no-op: released by tmp_pathbuf at end of scope */
+  }
+
+  tmp_pathbuf_allocator ();
+  tmp_pathbuf_allocator (tmp_pathbuf_allocator const &);
+  tmp_pathbuf_allocator & operator = (tmp_pathbuf_allocator const &);
+
+public:
+  /* use tmp_pathbuf of current stack frame */
+  tmp_pathbuf_allocator (tmp_pathbuf & tp)
+    : allocator_interface ()
+    , tp_ (tp)
+    , freemem_ ()
+    , freesize_ (0)
+  {}
+};
 
 static void
 set_dl_error (const char *str)
@@ -28,29 +96,6 @@ set_dl_error (const char *str)
   _my_tls.locals.dl_error = 1;
 }
 
-/* Look for an executable file given the name and the environment
-   variable to use for searching (eg., PATH); returns the full
-   pathname (static buffer) if found or NULL if not. */
-inline const char *
-check_path_access (const char *mywinenv, const char *name, path_conv& buf)
-{
-  return find_exec (name, buf, mywinenv, FE_NNF | FE_DLL);
-}
-
-/* Search LD_LIBRARY_PATH for dll, if it exists.  Search /usr/bin and /usr/lib
-   by default.  Return valid full path in path_conv real_filename. */
-static inline bool
-gfpod_helper (const char *name, path_conv &real_filename)
-{
-  if (strchr (name, '/'))
-    real_filename.check (name, PC_SYM_FOLLOW | PC_NULLEMPTY);
-  else if (!check_path_access ("LD_LIBRARY_PATH", name, real_filename))
-    check_path_access ("/usr/bin:/usr/lib", name, real_filename);
-  if (!real_filename.exists ())
-    real_filename.error = ENOENT;
-  return !real_filename.error;
-}
-
 static bool
 get_full_path_of_dll (const char* str, path_conv &real_filename)
 {
@@ -63,38 +108,52 @@ get_full_path_of_dll (const char* str, path_conv &real_filename)
       return false;		/* Yes.  Let caller deal with it. */
     }
 
-  tmp_pathbuf tp;
-  char *name = tp.c_get ();
+  tmp_pathbuf tp; /* single one per stack frame */
+  tmp_pathbuf_allocator allocator (tp);
+  pathfinder::basenamelist basenames (allocator);
 
-  strcpy (name, str);	/* Put it somewhere where we can manipulate it. */
+  const char *basename = strrchr (str, '/');
+  basename = basename ? basename + 1 : str;
 
-  char *basename = strrchr (name, '/');
-  basename = basename ? basename + 1 : name;
-  char *suffix = strrchr (name, '.');
-  if (suffix && suffix < basename)
-    suffix = NULL;
+  int baselen = str + len - basename;
+  const char *suffix = strrchr (basename, '.');
 
+  char const * ext = "";
+  int extlen = 0;
   /* Is suffix ".so"? */
   if (suffix && !strcmp (suffix, ".so"))
     {
       /* Does the file exist? */
-      if (gfpod_helper (name, real_filename))
-	return true;
+      basenames.appendv (basename, baselen, NULL);
       /* No, replace ".so" with ".dll". */
-      strcpy (suffix, ".dll");
+      baselen -= 3;
+      ext = ".dll";
+      extlen = 4;
     }
   /* Does the filename start with "lib"? */
   if (!strncmp (basename, "lib", 3))
     {
       /* Yes, replace "lib" with "cyg". */
-      strncpy (basename, "cyg", 3);
-      /* Does the file exist? */
-      if (gfpod_helper (name, real_filename))
-	return true;
+      basenames.appendv ("cyg", 3, basename+3, baselen-3, ext, extlen, NULL);
       /* No, revert back to "lib". */
-      strncpy (basename, "lib", 3);
     }
-  if (gfpod_helper (name, real_filename))
+  basenames.appendv (basename, baselen, ext, extlen, NULL);
+
+  pathfinder finder (allocator, basenames);
+
+  if (basename > str)
+    finder.add_searchdir (str, basename - 1 - str);
+  else
+    {
+      /* NOTE: The Windows loader (for linked dlls) does
+	 not use the LD_LIBRARY_PATH environment variable. */
+      finder.add_envsearchpath ("LD_LIBRARY_PATH");
+
+      /* Finally we better have some fallback. */
+      finder.add_lib_searchdir ("/usr/lib", -1);
+    }
+
+  if (finder.check_path_access (real_filename))
     return true;
 
   /* If nothing worked, create a relative path from the original incoming
diff --git a/winsup/cygwin/pathfinder.h b/winsup/cygwin/pathfinder.h
new file mode 100644
index 0000000..a9c74b3
--- /dev/null
+++ b/winsup/cygwin/pathfinder.h
@@ -0,0 +1,141 @@
+/* pathfinder.h: find one of multiple file names in path list
+
+This file is part of Cygwin.
+
+This software is a copyrighted work licensed under the terms of the
+Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
+details. */
+
+#include "vstrlist.h"
+
+#ifdef __cplusplus
+
+/* Search a list of directory names for first occurrence of a file,
+   which's file name matches one out of a list of file names.  */
+class pathfinder
+{
+public:
+  typedef vstrlist searchdirlist;
+  typedef vstrlist basenamelist;
+
+private:
+  pathfinder ();
+  pathfinder (pathfinder const &);
+  pathfinder & operator = (pathfinder const &);
+
+  basenamelist basenames_;
+  size_t       basenames_maxlen_;
+
+  /* Add to searchdirs_ with extra buffer for any basename we may search for.
+     This is an optimization for the loops in check_path_access method. */
+  searchdirlist searchdirs_;
+
+public:
+  ~pathfinder () {}
+
+  /* We need the basenames to search for first, to allow for optimized
+     memory allocation of each searchpath + longest basename combination.
+     The incoming list of basenames is emptied (ownership take over). */
+  pathfinder (allocator_interface & a, basenamelist & basenames)
+    : basenames_ (a)
+    , basenames_maxlen_ ()
+    , searchdirs_(a)
+  {
+    basenames_.swap(basenames);
+
+    for (basenamelist::iterator basename = basenames_.begin ();
+	 basename != basenames_.end ();
+	 ++ basename)
+      {
+	if (basenames_maxlen_ < basename->stringlength ())
+	  basenames_maxlen_ = basename->stringlength ();
+      }
+  }
+
+  void add_searchdir (const char *dir, int dirlen)
+  {
+      if (dirlen < 0)
+	dirlen = strlen (dir);
+
+      if (!dirlen)
+	return;
+
+      searchdirs_.appendv (dir, dirlen, "/", 1 + basenames_maxlen_, NULL);
+  }
+
+  void add_lib_searchdir (const char *dir, int dirlen)
+  {
+      if (dirlen < 0)
+	dirlen = strlen (dir);
+
+      if (!dirlen)
+	return;
+
+      /* Search "x/bin:x/lib" for "x/lib": As we don't have DT_RUNPATH,
+	 we have the real dll in /bin for library installed in /lib. */
+      if (dirlen >=4 && !strncmp (dir + dirlen - 4, "/lib", 4))
+	searchdirs_.appendv (dir, dirlen - 4, "/bin", 4, "/", 1 + basenames_maxlen_, NULL);
+
+      searchdirs_.appendv (dir, dirlen, "/", 1 + basenames_maxlen_, NULL);
+  }
+
+  void add_searchpath (const char *path)
+  {
+    while (path && *path)
+      {
+	const char *next = strchr (path, ':');
+	add_searchdir (path, next ? next - path : -1);
+	path = next ? next + 1 : next;
+      }
+  }
+
+  void add_lib_searchpath (const char *path)
+  {
+    while (path && *path)
+      {
+	const char *next = strchr (path, ':');
+	add_lib_searchdir (path, next ? next - path : -1);
+	path = next ? next + 1 : next;
+      }
+  }
+
+  void add_envsearchpath (const char *envpath)
+    {
+      add_searchpath (getenv (envpath));
+    }
+
+  void add_lib_envsearchpath (const char *envpath)
+    {
+      add_lib_searchpath (getenv (envpath));
+    }
+
+  /* Within each searchdir registered, try each registered basename to
+     find as executable.  Returns found dir/basename in real_filename.
+     Returns true when found. */
+  bool check_path_access (path_conv& real_filename)
+  {
+    for (basenamelist::iterator name = basenames_.begin ();
+	 name != basenames_.end ();
+	 ++name)
+      for (searchdirlist::buffer_iterator dir(searchdirs_.begin ());
+	   dir != searchdirs_.end ();
+	   ++dir)
+	{
+	  /* Complete the filename path to search for.
+	     We have allocated enough memory above.  */
+	  memcpy (dir->buffer () + dir->stringlength (),
+		  name->string (), name->stringlength () + 1);
+	  real_filename.check (dir->string (), PC_SYM_FOLLOW | PC_POSIX);
+	  if (real_filename.exists () && !real_filename.isdir ())
+	    {
+	      debug_printf ("found %s", dir->buffer ());
+	      return true;
+	    }
+	  debug_printf ("no %s", dir->buffer ());
+	}
+    real_filename.error = ENOENT;
+    return !real_filename.error;
+  }
+};
+
+#endif /* __cplusplus */
diff --git a/winsup/cygwin/vstrlist.h b/winsup/cygwin/vstrlist.h
new file mode 100644
index 0000000..241d8aa
--- /dev/null
+++ b/winsup/cygwin/vstrlist.h
@@ -0,0 +1,371 @@
+/* vstrlist.h: class vstrlist
+
+This file is part of Cygwin.
+
+This software is a copyrighted work licensed under the terms of the
+Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
+details. */
+
+#ifdef __cplusplus
+
+struct allocator_interface
+{
+  virtual void * alloc (size_t) = 0;
+  virtual void free (void *) = 0;
+};
+
+
+/* The allocated_type makes sure to use the free () method of the
+   same allocator_interface than the alloc () method was used of.
+
+   Stores the allocator_interface address before the real object,
+   to hide it from (construction & destruction of) real object.  */
+class allocated_type
+{
+  union allocator_store
+  {
+    allocator_interface * allocator_;
+    char alignment_[8];
+
+    union pointer
+    {
+      void            * vptr;
+      allocator_store * real;
+    };
+  };
+
+public:
+  void * operator new (size_t class_size, allocator_interface & allocator)
+  {
+    allocator_store::pointer astore;
+    astore.vptr = allocator.alloc (sizeof (allocator_store) + class_size);
+    astore.real->allocator_ = &allocator;
+    ++ astore.real;
+    return astore.vptr;
+  }
+
+  void operator delete (void * p)
+  {
+    allocator_store::pointer astore;
+    astore.vptr = p;
+    -- astore.real;
+    astore.real->allocator_->free (astore.vptr);
+  }
+};
+
+
+/* Double linked list of char arrays, each being a string buffer,
+   which's final buffer size and initial string content is defined
+   by a NULL terminated variable argument list of STRING+LEN pairs,
+   where each STRING (up to LEN) is concatenated for the initial
+   string buffer content, and each LEN is added to the final size
+   of the allocated string buffer.
+   If LEN is -1, strlen(STRING) is used for LEN.
+
+   Needs:
+     An implementation of the allocator_interface.
+
+   Provides:
+     iterator:
+       short name for the string_iterator
+     string_iterator:
+       provides readonly access via member methods:
+	 string (): readonly string buffer
+	 stringlength (): length (readonly) of initial string
+     buffer_iterator:
+       extends string_iterator
+       provides writeable access via member methods:
+	 buffer (): writeable string buffer
+	 bufferlength (): length (readonly) of allocated buffer
+
+   Usage sample:
+     char * text = "snipe";
+     vstrlist l;
+     l.appendv (text, 4, text+3, 2, "", 2, NULL);
+     buffer_iterator it (l.begin ());
+     strcpy (it->buffer () + it->stringlength (), "ts");
+     printf ("Sample result is: '%s'", it->string ());
+   Sample result is: 'snippets' */
+class vstrlist
+{
+public:
+  class member
+    : public allocated_type
+  {
+    friend class vstrlist;
+    friend class string_iterator;
+
+    member * prev_;
+    member * next_;
+    size_t   bufferlength_;
+    size_t   stringlength_;
+    char     buffer_[1]; /* we always have space for the trailing zero */
+
+    /* no copy, just swap */
+    member (member const &);
+    member & operator = (member const &);
+
+    /* anchor */
+    void * operator new (size_t class_size, allocator_interface & allocator)
+    {
+      return allocated_type::operator new (class_size, allocator);
+    }
+
+    /* anchor */
+    member ()
+      : allocated_type ()
+      , prev_ (this)
+      , next_ (this)
+      , bufferlength_ (0)
+      , stringlength_ (0)
+      , buffer_ ()
+    {}
+
+    /* entry: determine memory size from args */
+    void * operator new (size_t class_size, allocator_interface & allocator,
+			 char const * part0, va_list parts)
+    {
+      char const * part = part0;
+      va_list partsdup;
+      va_copy (partsdup, parts);
+      while (part)
+	{
+	  int partlen = va_arg (partsdup, int);
+	  if (partlen < 0)
+	    partlen = strlen (part);
+	  class_size += partlen;
+	  part = va_arg (partsdup, char const *);
+	}
+      va_end (partsdup);
+
+      return allocated_type::operator new (class_size, allocator);
+    }
+
+    /* entry: instantly insert into list */
+    member (member * before, char const * part0, va_list parts)
+      : allocated_type ()
+      , prev_ (NULL)
+      , next_ (NULL)
+      , bufferlength_ (0)
+      , stringlength_ ()
+      , buffer_ ()
+    {
+      prev_ = before->prev_;
+      next_ = before;
+
+      prev_->next_ = this;
+      next_->prev_ = this;
+
+      char * dest = buffer_;
+      char const * part = part0;
+      va_list partsdup;
+      va_copy (partsdup, parts);
+      while (part)
+	{
+	  int partlen = va_arg (partsdup, int);
+	  if (partlen < 0)
+	    {
+	      char * old = dest;
+	      dest = stpcpy (old, part);
+	      partlen = dest - old;
+	    }
+	  else
+	    dest = stpncpy (dest, part, partlen);
+	  bufferlength_ += partlen;
+	  part = va_arg (partsdup, const char *);
+	}
+      va_end (partsdup);
+      *dest = (char)0;
+      stringlength_ = dest - buffer_;
+    }
+
+    /* remove entry from list */
+    ~member ()
+    {
+      member * next = next_;
+      member * prev = prev_;
+      if (next)
+	next->prev_ = prev;
+      if (prev)
+	prev->next_ = next;
+      prev_ = NULL;
+      next_ = NULL;
+    }
+
+  public:
+    member const * next () const { return next_; }
+    member       * next ()       { return next_; }
+    member const * prev () const { return next_; }
+    member       * prev ()       { return next_; }
+
+    /* readonly access */
+    char const * string () const { return buffer_; }
+    size_t stringlength () const { return stringlength_; }
+
+    /* writeable access */
+    char       * buffer ()       { return buffer_; }
+    size_t bufferlength ()       { return bufferlength_; }
+  };
+
+  /* readonly access */
+  class string_iterator
+  {
+    friend class vstrlist;
+    friend class buffer_iterator;
+
+    member * current_;
+
+    string_iterator ();
+
+    string_iterator (member * current)
+      : current_ (current)
+    {}
+
+  public:
+    string_iterator (string_iterator const & rhs)
+      : current_ (rhs.current_)
+    {}
+
+    string_iterator & operator = (string_iterator const & rhs)
+    {
+      current_ = rhs.current_;
+      return *this;
+    }
+
+    string_iterator & operator ++ ()
+    {
+      current_ = current_->next ();
+      return *this;
+    }
+
+    string_iterator operator ++ (int)
+    {
+      string_iterator ret (*this);
+      current_ = current_->next ();
+      return ret;
+    }
+
+    string_iterator & operator -- ()
+    {
+      current_ = current_->prev ();
+      return *this;
+    }
+
+    string_iterator operator -- (int)
+    {
+      string_iterator ret (*this);
+      current_ = current_->prev ();
+      return ret;
+    }
+
+    bool operator == (string_iterator const & rhs) const
+    {
+      return current_ == rhs.current_;
+    }
+
+    bool operator != (string_iterator const & rhs) const
+    {
+      return current_ != rhs.current_;
+    }
+
+    /* readonly member access */
+    member const & operator *  () const { return *current_; }
+    member const * operator -> () const { return  current_; }
+
+    void remove ()
+    {
+      member * old = current_;
+      ++ *this;
+      delete old;
+    }
+  };
+
+  /* writeable access */
+  class buffer_iterator
+    : public string_iterator
+  {
+  public:
+    explicit /* can be used with vstrlist.begin () */
+    buffer_iterator (string_iterator const & begin)
+      : string_iterator (begin)
+    {}
+
+    buffer_iterator (buffer_iterator const & rhs)
+      : string_iterator (rhs)
+    {}
+
+    buffer_iterator & operator = (buffer_iterator const & rhs)
+    {
+      string_iterator::operator = (rhs);
+      return *this;
+    }
+
+    /* writeable member access */
+    member & operator *  () const { return *current_; }
+    member * operator -> () const { return  current_; }
+  };
+
+private:
+  allocator_interface & allocator_;
+  member              * anchor_;
+
+  /* not without an allocator */
+  vstrlist ();
+
+  /* no copy, just swap () */
+  vstrlist (vstrlist const &);
+  vstrlist & operator = (vstrlist const &);
+
+public:
+  /* iterator is the string_iterator */
+  typedef class string_iterator iterator;
+
+  iterator  begin () { return iterator (anchor_->next ()); }
+  iterator  end   () { return iterator (anchor_         ); }
+  iterator rbegin () { return iterator (anchor_->prev ()); }
+  iterator rend   () { return iterator (anchor_         ); }
+
+  vstrlist (allocator_interface & a)
+    : allocator_ (a)
+    , anchor_ (NULL) /* exception safety */
+  {
+    anchor_ = new (allocator_) member ();
+  }
+
+  ~vstrlist ()
+  {
+    if (anchor_ != NULL)
+      {
+	for (iterator it = begin (); it != end (); it.remove ());
+	delete anchor_;
+      }
+  }
+
+  void swap (vstrlist & that)
+  {
+    allocator_interface & a = allocator_;
+    member              * m = anchor_;
+    allocator_ = that.allocator_;
+    anchor_    = that.anchor_;
+    that.allocator_ = a;
+    that.anchor_    = m;
+  }
+
+  string_iterator appendv (char const * part0, va_list parts)
+  {
+    member * ret = new (allocator_, part0, parts)
+		       member (anchor_, part0, parts);
+    return string_iterator (ret);
+  }
+
+  string_iterator appendv (char const * part0, ...)
+  {
+    va_list parts;
+    va_start (parts, part0);
+    string_iterator ret = appendv (part0, parts);
+    va_end (parts);
+    return ret;
+  }
+};
+
+#endif /* __cplusplus */
-- 
2.8.3

>From 33d5cd5cd1dc188662d6cd18b3e8f4d902559bcf Mon Sep 17 00:00:00 2001
From: Michael Haubenwallner <michael.haubenwallner@ssi-schaefer.com>
Date: Thu, 25 Aug 2016 14:14:55 +0200
Subject: [PATCH 2/4] dlopen (pathfinder): try each basename per dir

Rather than searching within all search dirs for one basename,
search for all basenames within one search dir.

pathfinder.h (check_path_access): Interchange dir- and basename-loops.
---
 winsup/cygwin/pathfinder.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/winsup/cygwin/pathfinder.h b/winsup/cygwin/pathfinder.h
index a9c74b3..e81d00e 100644
--- a/winsup/cygwin/pathfinder.h
+++ b/winsup/cygwin/pathfinder.h
@@ -114,12 +114,12 @@ public:
      Returns true when found. */
   bool check_path_access (path_conv& real_filename)
   {
-    for (basenamelist::iterator name = basenames_.begin ();
-	 name != basenames_.end ();
-	 ++name)
-      for (searchdirlist::buffer_iterator dir(searchdirs_.begin ());
-	   dir != searchdirs_.end ();
-	   ++dir)
+    for (searchdirlist::buffer_iterator dir(searchdirs_.begin ());
+	 dir != searchdirs_.end ();
+	 ++dir)
+      for (basenamelist::iterator name = basenames_.begin ();
+	   name != basenames_.end ();
+	   ++name)
 	{
 	  /* Complete the filename path to search for.
 	     We have allocated enough memory above.  */
-- 
2.8.3


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