[PATCH 0/4] dlopen: improving the search algorithm

classic Classic list List threaded Threaded
21 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 0/4] dlopen: improving the search algorithm

Michael Haubenwallner-2
Hi Corinna,

based on the discussion on -developers list [1][2], here's a reworked
set of patches for dlopen, split along these features and fixes.

[1] https://cygwin.com/ml/cygwin-developers/2016-06/msg00000.html
[2] https://cygwin.com/ml/cygwin-developers/2016-08/msg00000.html

*) Switch to pathfinder class for path search iteration, without any
deliberate change in behaviour, but with the pathfinder::criterion
interface to allow for more generic use eventually.

*) Fix search order to search all basenames per one directory
rather than searching all directories per one basename.

*) For dlopen ("/path/lib/libz.so"), search "/path/bin/" first when
the executable calling is located in "/path/bin/".  This is for [3].
[3] https://cygwin.com/ml/cygwin-developers/2016-08/msg00020.html

*) Consequently, dlopen ("libAPP.so") without an explicit path
should search the executable's directory first, like the Windows
loader does for linked dlls.

Topics dropped for now:

*) Extra GetModuleHandleEx is not that necessary to me, as it[4]
turns out that LoadLibrary detects a dll as already loaded even
if the underlying dll file has been replaced (renamed actually).
[4] https://cygwin.com/ml/cygwin-developers/2016-08/msg00023.html

*) Add PATH environment variable to list of searchdirs should not
be necessary to me when the executable dir is added.

Thank you!
/haubi/

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Michael Haubenwallner-2
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.
---
 winsup/cygwin/dlfcn.cc     | 310 ++++++++++++++++++++++++-------------
 winsup/cygwin/pathfinder.h | 208 +++++++++++++++++++++++++
 winsup/cygwin/vstrlist.h   | 373 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 782 insertions(+), 109 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..e592512 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,84 +96,61 @@ 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)
+/* Identify basename and baselen within name,
+   return true if there is a dir in name. */
+static bool
+spot_basename (const char * &basename, int &baselen, const char * name)
 {
-  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;
+  basename = strrchr (name, '/');
+  basename = basename ? basename + 1 : name;
+  baselen = name + strlen (name) - basename;
+  return basename > name;
 }
 
-static bool
-get_full_path_of_dll (const char* str, path_conv &real_filename)
+/* Setup basenames using basename and baselen,
+   return true if basenames do have some suffix. */
+static void
+collect_basenames (pathfinder::basenamelist & basenames,
+   const char * basename, int baselen)
 {
-  int len = strlen (str);
+  /* like strrchr (basename, '.'), but limited to baselen */
+  const char *suffix = basename + baselen;
+  while (--suffix >= basename)
+    if (*suffix == '.')
+      break;
 
-  /* empty string? */
-  if (len == 0)
+  int suffixlen;
+  if (suffix >= basename)
+    suffixlen = basename + baselen - suffix;
+  else
     {
-      set_errno (EINVAL);
-      return false; /* Yes.  Let caller deal with it. */
+      suffixlen = 0;
+      suffix = NULL;
     }
 
-  tmp_pathbuf tp;
-  char *name = tp.c_get ();
-
-  strcpy (name, str); /* Put it somewhere where we can manipulate it. */
+  char const * ext = "";
+  /* Without some suffix, reserve space for a trailing dot to override
+     GetModuleHandleExA's automatic adding of the ".dll" suffix. */
+  int extlen = suffix ? 0 : 1;
 
-  char *basename = strrchr (name, '/');
-  basename = basename ? basename + 1 : name;
-  char *suffix = strrchr (name, '.');
-  if (suffix && suffix < basename)
-    suffix = NULL;
-
-  /* Is suffix ".so"? */
-  if (suffix && !strcmp (suffix, ".so"))
+  /* If we have the ".so" suffix, ... */
+  if (suffixlen == 3 && !strncmp (suffix, ".so", 3))
     {
-      /* Does the file exist? */
-      if (gfpod_helper (name, real_filename))
- return true;
-      /* No, replace ".so" with ".dll". */
-      strcpy (suffix, ".dll");
+      /* ... keep the basename with original suffix, before ... */
+      basenames.appendv (basename, baselen, NULL);
+      /* ... replacing the ".so" with the ".dll" suffix. */
+      baselen -= 3;
+      ext = ".dll";
+      extlen = 4;
     }
-  /* Does the filename start with "lib"? */
+  /* If the basename starts 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;
-      /* No, revert back to "lib". */
-      strncpy (basename, "lib", 3);
+      /* ... replace "lib" with "cyg", before ... */
+      basenames.appendv ("cyg", 3, basename+3, baselen-3, ext, extlen, NULL);
     }
-  if (gfpod_helper (name, real_filename))
-    return true;
-
-  /* If nothing worked, create a relative path from the original incoming
-     filename and let LoadLibrary search for it using the system default
-     DLL search path. */
-  real_filename.check (str, PC_SYM_FOLLOW | PC_NOFULL | PC_NULLEMPTY);
-  if (!real_filename.error)
-    return true;
-
-  set_errno (real_filename.error);
-  return false;
+  /* ... using original basename with new suffix. */
+  basenames.appendv (basename, baselen, ext, extlen, NULL);
 }
 
 extern "C" void *
@@ -113,64 +158,111 @@ dlopen (const char *name, int flags)
 {
   void *ret = NULL;
 
-  if (name == NULL)
-    {
-      ret = (void *) GetModuleHandle (NULL); /* handle for the current module */
-      if (!ret)
- __seterrno ();
-    }
-  else
+  do
     {
+      if (name == NULL || *name == '\0')
+ {
+  ret = (void *) GetModuleHandle (NULL); /* handle for the current module */
+  if (!ret)
+    __seterrno ();
+  break;
+ }
+
+      DWORD gmheflags = (flags & RTLD_NODELETE)
+      ?  GET_MODULE_HANDLE_EX_FLAG_PIN
+      : 0;
+
+      tmp_pathbuf tp; /* single one per stack frame */
+      tmp_pathbuf_allocator allocator (tp);
+      pathfinder::basenamelist basenames (allocator);
+
+      const char *basename;
+      int baselen;
+      bool have_dir = spot_basename (basename, baselen, name);
+      collect_basenames (basenames, basename, baselen);
+
       /* handle for the named library */
-      path_conv pc;
-      if (get_full_path_of_dll (name, pc))
+      path_conv real_filename;
+      wchar_t *wpath = tp.w_get ();
+
+      pathfinder finder (allocator, basenames); /* eats basenames */
+
+      if (have_dir)
+ {
+  /* search the specified dir */
+  finder.add_searchdir (name, basename - 1 - name);
+ }
+      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_searchdir ("/usr/bin", 8);
+  finder.add_searchdir ("/usr/lib", 8);
+ }
+
+      /* now search the file system */
+      if (!finder.find (pathfinder::
+ exists_and_not_dir (real_filename,
+    PC_SYM_FOLLOW | PC_POSIX)))
  {
-  tmp_pathbuf tp;
-  wchar_t *path = tp.w_get ();
+  /* If nothing worked, create a relative path from the original
+     incoming filename and let LoadLibrary search for it using the
+     system default DLL search path. */
+  real_filename.check (name, PC_SYM_FOLLOW | PC_NOFULL | PC_NULLEMPTY);
+  if (real_filename.error)
+    break;
+ }
 
-  pc.get_wide_win32_path (path);
-  /* Check if the last path component contains a dot.  If so,
-     leave the filename alone.  Otherwise add a trailing dot
-     to override LoadLibrary's automatic adding of a ".dll" suffix. */
-  wchar_t *last_bs = wcsrchr (path, L'\\') ?: path;
-  if (last_bs && !wcschr (last_bs, L'.'))
-    wcscat (last_bs, L".");
+      real_filename.get_wide_win32_path (wpath);
+      /* Check if the last path component contains a dot.  If so,
+ leave the filename alone.  Otherwise add a trailing dot
+ to override LoadLibrary's automatic adding of a ".dll" suffix. */
+      wchar_t *last_bs = wcsrchr (wpath, L'\\') ?: wpath;
+      if (last_bs && !wcschr (last_bs, L'.'))
+ wcscat (last_bs, L".");
+
+      if (flags & RTLD_NOLOAD)
+ {
+  GetModuleHandleExW (gmheflags, wpath, (HMODULE *) &ret);
+  if (ret)
+    break;
+ }
 
 #ifndef __x86_64__
-  /* Workaround for broken DLLs built against Cygwin versions 1.7.0-49
-     up to 1.7.0-57.  They override the cxx_malloc pointer in their
-     DLL initialization code even if loaded dynamically.  This is a
-     no-no since a later dlclose lets cxx_malloc point into nirvana.
-     The below kludge "fixes" that by reverting the original cxx_malloc
-     pointer after LoadLibrary.  This implies that their overrides
-     won't be applied; that's OK.  All overrides should be present at
-     final link time, as Windows doesn't allow undefined references;
-     it would actually be wrong for a dlopen'd DLL to opportunistically
-     override functions in a way that wasn't known then.  We're not
-     going to try and reproduce the full ELF dynamic loader here!  */
-
-  /* Store original cxx_malloc pointer. */
-  struct per_process_cxx_malloc *tmp_malloc;
-  tmp_malloc = __cygwin_user_data.cxx_malloc;
+      /* Workaround for broken DLLs built against Cygwin versions 1.7.0-49
+ up to 1.7.0-57.  They override the cxx_malloc pointer in their
+ DLL initialization code even if loaded dynamically.  This is a
+ no-no since a later dlclose lets cxx_malloc point into nirvana.
+ The below kludge "fixes" that by reverting the original cxx_malloc
+ pointer after LoadLibrary.  This implies that their overrides
+ won't be applied; that's OK.  All overrides should be present at
+ final link time, as Windows doesn't allow undefined references;
+ it would actually be wrong for a dlopen'd DLL to opportunistically
+ override functions in a way that wasn't known then.  We're not
+ going to try and reproduce the full ELF dynamic loader here!  */
+
+      /* Store original cxx_malloc pointer. */
+      struct per_process_cxx_malloc *tmp_malloc;
+      tmp_malloc = __cygwin_user_data.cxx_malloc;
 #endif
 
-  if (flags & RTLD_NOLOAD)
-    GetModuleHandleExW (0, path, (HMODULE *) &ret);
-  else
-    ret = (void *) LoadLibraryW (path);
-  if (ret && (flags & RTLD_NODELETE))
-    GetModuleHandleExW (GET_MODULE_HANDLE_EX_FLAG_PIN, path,
- (HMODULE *) &ret);
+      ret = (void *) LoadLibraryW (wpath);
 
 #ifndef __x86_64__
-  /* Restore original cxx_malloc pointer. */
-  __cygwin_user_data.cxx_malloc = tmp_malloc;
+      /* Restore original cxx_malloc pointer. */
+      __cygwin_user_data.cxx_malloc = tmp_malloc;
 #endif
 
-  if (!ret)
-    __seterrno ();
- }
+      if (ret && gmheflags)
+ GetModuleHandleExW (gmheflags, wpath, (HMODULE *) &ret);
+
+      if (!ret)
+ __seterrno ();
     }
+  while (0);
 
   if (!ret)
     set_dl_error ("dlopen");
diff --git a/winsup/cygwin/pathfinder.h b/winsup/cygwin/pathfinder.h
new file mode 100644
index 0000000..bbba168
--- /dev/null
+++ b/winsup/cygwin/pathfinder.h
@@ -0,0 +1,208 @@
+/* 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::buffer_iterator basename (basenames_.begin ());
+ basename != basenames_.end ();
+ ++ basename)
+      {
+ if (basenames_maxlen_ < basename->bufferlength ())
+  basenames_maxlen_ = basename->bufferlength ();
+      }
+  }
+
+  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_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_envsearchpath (const char *envpath)
+    {
+      add_searchpath (getenv (envpath));
+    }
+
+
+  /* pathfinder::criterion_interface
+     Overload this test method when you need separate dir and basename.  */
+  struct criterion_interface
+  {
+    virtual char const * name () const { return NULL; }
+
+    virtual bool test (searchdirlist::iterator dir,
+       basenamelist::iterator name) const = 0;
+  };
+
+
+  /* pathfinder::simple_criterion_interface
+     Overload this test method when you need a single filename.  */
+  class simple_criterion_interface
+    : public criterion_interface
+  {
+    virtual bool test (searchdirlist::iterator dir,
+       basenamelist::iterator name) const
+    {
+      /* Complete the filename path to search for within dir,
+ We have allocated enough memory above.  */
+      searchdirlist::buffer_iterator dirbuf (dir);
+      memcpy (dirbuf->buffer () + dirbuf->stringlength (),
+      name->string (), name->stringlength () + 1);
+      bool ret = test (dirbuf->string ());
+      /* reset original dir */
+      dirbuf->buffer ()[dirbuf->stringlength ()] = '\0';
+      return ret;
+    }
+
+  public:
+    virtual bool test (const char * filename) const = 0;
+  };
+
+
+  /* pathfinder::path_conv_criterion_interface
+     Overload this test method when you need a path_conv. */
+  class path_conv_criterion_interface
+    : public simple_criterion_interface
+  {
+    path_conv mypc_;
+    path_conv & pc_;
+    unsigned opt_;
+
+    /* simple_criterion_interface */
+    virtual bool test (const char * filename) const
+    {
+      pc_.check (filename, opt_);
+      return test (pc_);
+    }
+
+  public:
+    path_conv_criterion_interface (unsigned opt = PC_SYM_FOLLOW)
+      : mypc_ ()
+      , pc_ (mypc_)
+      , opt_ (opt)
+    {}
+
+    path_conv_criterion_interface (path_conv & ret, unsigned opt = PC_SYM_FOLLOW)
+      : mypc_ ()
+      , pc_ (ret)
+      , opt_ (opt)
+    {}
+
+    virtual bool test (path_conv & pc) const = 0;
+  };
+
+
+  /* pathfinder::exists_and_not_dir
+     Test if path_conv argument does exist and is not a directory. */
+  struct exists_and_not_dir
+    : public path_conv_criterion_interface
+  {
+    virtual char const * name () const { return "exists and not dir"; }
+
+    exists_and_not_dir (path_conv & pc, unsigned opt = PC_SYM_FOLLOW)
+      : path_conv_criterion_interface (pc, opt)
+    {}
+
+    /* path_conv_criterion_interface */
+    virtual bool test (path_conv & pc) const
+    {
+      if (pc.exists () && !pc.isdir ())
+ return true;
+
+      pc.error = ENOENT;
+      return false;
+    }
+  };
+
+
+  /* Find the single dir + basename that matches criterion.
+
+     Calls criterion.test method for each registered dir + basename
+     until returning true:
+       Returns true with found_dir + found_basename set.
+     If criterion.test method never returns true:
+       Returns false, not modifying found_dir nor found_basename.  */
+  bool find (criterion_interface const & criterion,
+     searchdirlist::member const ** found_dir = NULL,
+     basenamelist::member const ** found_basename = NULL)
+  {
+    char const * critname = criterion.name ();
+    for (basenamelist::iterator name = basenames_.begin ();
+ name != basenames_.end ();
+ ++name)
+      for (searchdirlist::iterator dir(searchdirs_.begin ());
+   dir != searchdirs_.end ();
+   ++dir)
+ if (criterion.test (dir, name))
+  {
+    debug_printf ("(%s), take %s%s", critname,
+  dir->string(), name->string ());
+    if (found_dir)
+      *found_dir = dir.operator -> ();
+    if (found_basename)
+      *found_basename = name.operator -> ();
+    return true;
+  }
+ else
+  debug_printf ("not (%s), skip %s%s", critname,
+ dir->string(), name->string ());
+    return false;
+  }
+};
+
+#endif /* __cplusplus */
diff --git a/winsup/cygwin/vstrlist.h b/winsup/cygwin/vstrlist.h
new file mode 100644
index 0000000..d5ad717
--- /dev/null
+++ b/winsup/cygwin/vstrlist.h
@@ -0,0 +1,373 @@
+/* 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_;
+      if (bufferlength_ > stringlength_)
+ memset (++dest, 0, bufferlength_ - stringlength_);
+    }
+
+    /* 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.7.3

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 2/4] dlopen (pathfinder): try each basename per dir

Michael Haubenwallner-2
In reply to this post by Michael Haubenwallner-2
Rather than searching all search dirs per one basename,
search for all basenames within per 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 bbba168..c306604 100644
--- a/winsup/cygwin/pathfinder.h
+++ b/winsup/cygwin/pathfinder.h
@@ -182,12 +182,12 @@ public:
      basenamelist::member const ** found_basename = NULL)
   {
     char const * critname = criterion.name ();
-    for (basenamelist::iterator name = basenames_.begin ();
- name != basenames_.end ();
- ++name)
-      for (searchdirlist::iterator dir(searchdirs_.begin ());
-   dir != searchdirs_.end ();
-   ++dir)
+    for (searchdirlist::iterator dir(searchdirs_.begin ());
+ dir != searchdirs_.end ();
+ ++dir)
+      for (basenamelist::iterator name = basenames_.begin ();
+   name != basenames_.end ();
+   ++name)
  if (criterion.test (dir, name))
   {
     debug_printf ("(%s), take %s%s", critname,
--
2.7.3

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 3/4] dlopen: on x/lib search x/bin if exe is in x/bin

Michael Haubenwallner-2
In reply to this post by Michael Haubenwallner-2
citing https://cygwin.com/ml/cygwin-developers/2016-08/msg00020.html
> Consider the file /usr/bin/cygz.dll:
> - dlopen (libz.so)            success
> - dlopen (/usr/bin/libz.so)   success
> - dlopen (/usr/lib/libz.so)   fails

* dlfcn.c (dlopen): For dlopen("x/lib/N"), when the application
executable is in "x/bin/", search for "x/bin/N" before "x/lib/N".
---
 winsup/cygwin/dlfcn.cc | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/dlfcn.cc b/winsup/cygwin/dlfcn.cc
index e592512..f8b8743 100644
--- a/winsup/cygwin/dlfcn.cc
+++ b/winsup/cygwin/dlfcn.cc
@@ -153,6 +153,25 @@ collect_basenames (pathfinder::basenamelist & basenames,
   basenames.appendv (basename, baselen, ext, extlen, NULL);
 }
 
+/* Identify dir of current executable into exedirbuf using wpathbuf buffer.
+   Return length of exedirbuf on success, or zero on error. */
+static int
+get_exedir (char * exedirbuf, wchar_t * wpathbuf)
+{
+  /* Unless we have a special cygwin loader, there is no such thing like
+     DT_RUNPATH on Windows we can use to search for dlls, except for the
+     directory of the main executable. */
+  GetModuleFileNameW (NULL, wpathbuf, NT_MAX_PATH);
+  wchar_t * lastwsep = wcsrchr (wpathbuf, L'\\');
+  if (!lastwsep)
+    return 0;
+  *lastwsep = L'\0';
+  *exedirbuf = '\0';
+  if (cygwin_conv_path (CCP_WIN_W_TO_POSIX, wpathbuf, exedirbuf, NT_MAX_PATH))
+    return 0;
+  return strlen (exedirbuf);
+}
+
 extern "C" void *
 dlopen (const char *name, int flags)
 {
@@ -184,13 +203,28 @@ dlopen (const char *name, int flags)
       /* handle for the named library */
       path_conv real_filename;
       wchar_t *wpath = tp.w_get ();
+      char *cpath = tp.c_get ();
 
       pathfinder finder (allocator, basenames); /* eats basenames */
 
       if (have_dir)
  {
+  int dirlen = basename - 1 - name;
+
+  /* if the specified dir is x/lib, and the current executable
+     dir is x/bin, do the /lib -> /bin mapping, which is the
+     same actually as adding the executable dir */
+  if (dirlen >= 4 && !strncmp (name + dirlen - 4, "/lib", 4))
+    {
+      int exedirlen = get_exedir (cpath, wpath);
+      if (exedirlen == dirlen &&
+  !strncmp (cpath, name, dirlen - 4) &&
+  !strcmp (cpath + dirlen - 4, "/bin"))
+ finder.add_searchdir (cpath, exedirlen);
+    }
+
   /* search the specified dir */
-  finder.add_searchdir (name, basename - 1 - name);
+  finder.add_searchdir (name, dirlen);
  }
       else
  {
--
2.7.3

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 4/4] dlopen: on unspecified lib dir search exe dir

Michael Haubenwallner-2
In reply to this post by Michael Haubenwallner-2
Applications installed to some prefix like /opt/application do expect
dlopen("libAPP.so") to load "/opt/application/bin/cygAPP.dll", which
is similar to "/opt/application/lib/libAPP.so" on Linux.

See also https://cygwin.com/ml/cygwin-developers/2016-08/msg00020.html

* dlfcn.cc (dlopen): For dlopen("N"), search directory where the
application executable is in.
---
 winsup/cygwin/dlfcn.cc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/winsup/cygwin/dlfcn.cc b/winsup/cygwin/dlfcn.cc
index f8b8743..974092e 100644
--- a/winsup/cygwin/dlfcn.cc
+++ b/winsup/cygwin/dlfcn.cc
@@ -232,6 +232,12 @@ dlopen (const char *name, int flags)
      not use the LD_LIBRARY_PATH environment variable. */
   finder.add_envsearchpath ("LD_LIBRARY_PATH");
 
+  /* Search the current executable's directory like
+     the Windows loader does for linked dlls. */
+  int exedirlen = get_exedir (cpath, wpath);
+  if (exedirlen)
+    finder.add_searchdir (cpath, exedirlen);
+
   /* Finally we better have some fallback. */
   finder.add_searchdir ("/usr/bin", 8);
   finder.add_searchdir ("/usr/lib", 8);
--
2.7.3

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Corinna Vinschen-2
In reply to this post by Michael Haubenwallner-2
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

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Michael Haubenwallner-2
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

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

Thanks!
/haubi/

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 3/4] dlopen: on x/lib search x/bin if exe is in x/bin

Corinna Vinschen-2
In reply to this post by Michael Haubenwallner-2
Hi Michael,

On Aug 31 20:07, Michael Haubenwallner wrote:

> citing https://cygwin.com/ml/cygwin-developers/2016-08/msg00020.html
> > Consider the file /usr/bin/cygz.dll:
> > - dlopen (libz.so)            success
> > - dlopen (/usr/bin/libz.so)   success
> > - dlopen (/usr/lib/libz.so)   fails
>
> * dlfcn.c (dlopen): For dlopen("x/lib/N"), when the application
> executable is in "x/bin/", search for "x/bin/N" before "x/lib/N".
> ---
>  winsup/cygwin/dlfcn.cc | 36 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/winsup/cygwin/dlfcn.cc b/winsup/cygwin/dlfcn.cc
> index e592512..f8b8743 100644
> --- a/winsup/cygwin/dlfcn.cc
> +++ b/winsup/cygwin/dlfcn.cc
> @@ -153,6 +153,25 @@ collect_basenames (pathfinder::basenamelist & basenames,
>    basenames.appendv (basename, baselen, ext, extlen, NULL);
>  }
>  
> +/* Identify dir of current executable into exedirbuf using wpathbuf buffer.
> +   Return length of exedirbuf on success, or zero on error. */
> +static int
> +get_exedir (char * exedirbuf, wchar_t * wpathbuf)
> +{
> +  /* Unless we have a special cygwin loader, there is no such thing like
> +     DT_RUNPATH on Windows we can use to search for dlls, except for the
> +     directory of the main executable. */
> +  GetModuleFileNameW (NULL, wpathbuf, NT_MAX_PATH);
> +  wchar_t * lastwsep = wcsrchr (wpathbuf, L'\\');
> +  if (!lastwsep)
> +    return 0;
> +  *lastwsep = L'\0';
> +  *exedirbuf = '\0';
> +  if (cygwin_conv_path (CCP_WIN_W_TO_POSIX, wpathbuf, exedirbuf, NT_MAX_PATH))
> +    return 0;
> +  return strlen (exedirbuf);
> +}
You could just use the global variable program_invocation_name.  If in
doubt, use the Windows path global_progname and convert it to full POSIX
via cygwin_conv_path.

>  extern "C" void *
>  dlopen (const char *name, int flags)
>  {
> @@ -184,13 +203,28 @@ dlopen (const char *name, int flags)
>        /* handle for the named library */
>        path_conv real_filename;
>        wchar_t *wpath = tp.w_get ();
> +      char *cpath = tp.c_get ();
>  
>        pathfinder finder (allocator, basenames); /* eats basenames */
>  
>        if (have_dir)
>   {
> +  int dirlen = basename - 1 - name;
> +
> +  /* if the specified dir is x/lib, and the current executable
> +     dir is x/bin, do the /lib -> /bin mapping, which is the
> +     same actually as adding the executable dir */
> +  if (dirlen >= 4 && !strncmp (name + dirlen - 4, "/lib", 4))
> +    {
> +      int exedirlen = get_exedir (cpath, wpath);
> +      if (exedirlen == dirlen &&
> +  !strncmp (cpath, name, dirlen - 4) &&
> +  !strcmp (cpath + dirlen - 4, "/bin"))
> + finder.add_searchdir (cpath, exedirlen);
> +    }
> +
>    /* search the specified dir */
> -  finder.add_searchdir (name, basename - 1 - name);
> +  finder.add_searchdir (name, dirlen);
>   }
>        else
>   {
> --
> 2.7.3
Rest looks ok.


Thanks,
Corinna

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

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Corinna Vinschen-2
In reply to this post by Michael Haubenwallner-2
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

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 2/4] dlopen (pathfinder): try each basename per dir

Corinna Vinschen-2
In reply to this post by Michael Haubenwallner-2
On Aug 31 20:07, Michael Haubenwallner wrote:

> Rather than searching all search dirs per one basename,
> search for all basenames within per 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 bbba168..c306604 100644
> --- a/winsup/cygwin/pathfinder.h
> +++ b/winsup/cygwin/pathfinder.h
> @@ -182,12 +182,12 @@ public:
>       basenamelist::member const ** found_basename = NULL)
>    {
>      char const * critname = criterion.name ();
> -    for (basenamelist::iterator name = basenames_.begin ();
> - name != basenames_.end ();
> - ++name)
> -      for (searchdirlist::iterator dir(searchdirs_.begin ());
> -   dir != searchdirs_.end ();
> -   ++dir)
> +    for (searchdirlist::iterator dir(searchdirs_.begin ());
> + dir != searchdirs_.end ();
> + ++dir)
> +      for (basenamelist::iterator name = basenames_.begin ();
> +   name != basenames_.end ();
> +   ++name)
>   if (criterion.test (dir, name))
>    {
>      debug_printf ("(%s), take %s%s", critname,
> --
> 2.7.3
This one's fine, of course.


Thanks,
Corinna

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

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 4/4] dlopen: on unspecified lib dir search exe dir

Corinna Vinschen-2
In reply to this post by Michael Haubenwallner-2
On Aug 31 20:07, Michael Haubenwallner wrote:

> Applications installed to some prefix like /opt/application do expect
> dlopen("libAPP.so") to load "/opt/application/bin/cygAPP.dll", which
> is similar to "/opt/application/lib/libAPP.so" on Linux.
>
> See also https://cygwin.com/ml/cygwin-developers/2016-08/msg00020.html
>
> * dlfcn.cc (dlopen): For dlopen("N"), search directory where the
> application executable is in.
> ---
>  winsup/cygwin/dlfcn.cc | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/winsup/cygwin/dlfcn.cc b/winsup/cygwin/dlfcn.cc
> index f8b8743..974092e 100644
> --- a/winsup/cygwin/dlfcn.cc
> +++ b/winsup/cygwin/dlfcn.cc
> @@ -232,6 +232,12 @@ dlopen (const char *name, int flags)
>       not use the LD_LIBRARY_PATH environment variable. */
>    finder.add_envsearchpath ("LD_LIBRARY_PATH");
>  
> +  /* Search the current executable's directory like
> +     the Windows loader does for linked dlls. */
> +  int exedirlen = get_exedir (cpath, wpath);
> +  if (exedirlen)
> +    finder.add_searchdir (cpath, exedirlen);
> +
>    /* Finally we better have some fallback. */
>    finder.add_searchdir ("/usr/bin", 8);
>    finder.add_searchdir ("/usr/lib", 8);
> --
> 2.7.3
Still not quite sure if that's the right thing to do...


Corinna

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

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Michael Haubenwallner-2
In reply to this post by Corinna Vinschen-2
On 09/01/2016 04:03 PM, Corinna Vinschen wrote:

> On Sep  1 13:05, Michael Haubenwallner wrote:
>> On 08/31/2016 09:12 PM, Corinna Vinschen wrote:
>>> 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.

The idea to build the basenames list before constructing pathfinder
is that members of the searchdirs list reserve space for the maxlen
of basenames:  This implies that the basenames list must not change
after the first searchdir was registered.

To make sure this doesn't happen I prefer to not provide such an API
at all, rather than to check within some pathfinder::add_basename ()
method and abort if there is some searchdir registered already.

> Given that basenames is a member,
> you can do the allocator stuff completely inside the pathfinder class.

Moving the allocator into pathfinder would work then, but still the
tmp_pathbuf instance to use has to be provided as reference.

/haubi/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 3/4] dlopen: on x/lib search x/bin if exe is in x/bin

Michael Haubenwallner-2
In reply to this post by Corinna Vinschen-2
Hi Corinna,

On 09/01/2016 03:32 PM, Corinna Vinschen wrote:

> On Aug 31 20:07, Michael Haubenwallner wrote:
>> citing https://cygwin.com/ml/cygwin-developers/2016-08/msg00020.html
>>> Consider the file /usr/bin/cygz.dll:
>>> - dlopen (libz.so)            success
>>> - dlopen (/usr/bin/libz.so)   success
>>> - dlopen (/usr/lib/libz.so)   fails
>>
>> * dlfcn.c (dlopen): For dlopen("x/lib/N"), when the application
>> executable is in "x/bin/", search for "x/bin/N" before "x/lib/N".
>> ---
>>  winsup/cygwin/dlfcn.cc | 36 +++++++++++++++++++++++++++++++++++-
>>  1 file changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/winsup/cygwin/dlfcn.cc b/winsup/cygwin/dlfcn.cc
>> index e592512..f8b8743 100644
>> --- a/winsup/cygwin/dlfcn.cc
>> +++ b/winsup/cygwin/dlfcn.cc
>> @@ -153,6 +153,25 @@ collect_basenames (pathfinder::basenamelist & basenames,
>>    basenames.appendv (basename, baselen, ext, extlen, NULL);
>>  }
>>  
>> +/* Identify dir of current executable into exedirbuf using wpathbuf buffer.
>> +   Return length of exedirbuf on success, or zero on error. */
>> +static int
>> +get_exedir (char * exedirbuf, wchar_t * wpathbuf)
>> +{
>> +  /* Unless we have a special cygwin loader, there is no such thing like
>> +     DT_RUNPATH on Windows we can use to search for dlls, except for the
>> +     directory of the main executable. */
>> +  GetModuleFileNameW (NULL, wpathbuf, NT_MAX_PATH);
>> +  wchar_t * lastwsep = wcsrchr (wpathbuf, L'\\');
>> +  if (!lastwsep)
>> +    return 0;
>> +  *lastwsep = L'\0';
>> +  *exedirbuf = '\0';
>> +  if (cygwin_conv_path (CCP_WIN_W_TO_POSIX, wpathbuf, exedirbuf, NT_MAX_PATH))
>> +    return 0;
>> +  return strlen (exedirbuf);
>> +}
>
> You could just use the global variable program_invocation_name.  If in
> doubt, use the Windows path global_progname and convert it to full POSIX
> via cygwin_conv_path.
Patch updated, using global_progname now.

>>  extern "C" void *
>>  dlopen (const char *name, int flags)
>>  {
>> @@ -184,13 +203,28 @@ dlopen (const char *name, int flags)
>>        /* handle for the named library */
>>        path_conv real_filename;
>>        wchar_t *wpath = tp.w_get ();
>> +      char *cpath = tp.c_get ();
>>  
>>        pathfinder finder (allocator, basenames); /* eats basenames */
>>  
>>        if (have_dir)
>>   {
>> +  int dirlen = basename - 1 - name;
>> +
>> +  /* if the specified dir is x/lib, and the current executable
>> +     dir is x/bin, do the /lib -> /bin mapping, which is the
>> +     same actually as adding the executable dir */
>> +  if (dirlen >= 4 && !strncmp (name + dirlen - 4, "/lib", 4))
>> +    {
>> +      int exedirlen = get_exedir (cpath, wpath);
>> +      if (exedirlen == dirlen &&
>> +  !strncmp (cpath, name, dirlen - 4) &&
>> +  !strcmp (cpath + dirlen - 4, "/bin"))
>> + finder.add_searchdir (cpath, exedirlen);
>> +    }
>> +
>>    /* search the specified dir */
>> -  finder.add_searchdir (name, basename - 1 - name);
>> +  finder.add_searchdir (name, dirlen);
>>   }
>>        else
>>   {
>> --
>> 2.7.3
>
> Rest looks ok.
Thanks!
/haubi/


0003-dlopen-on-x-lib-search-x-bin-if-exe-is-in-x-bin.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Corinna Vinschen-2
In reply to this post by Michael Haubenwallner-2
On Sep  2 10:05, Michael Haubenwallner wrote:

> On 09/01/2016 04:03 PM, Corinna Vinschen wrote:
> > On Sep  1 13:05, Michael Haubenwallner wrote:
> >> On 08/31/2016 09:12 PM, Corinna Vinschen wrote:
> >>> 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.
>
> The idea to build the basenames list before constructing pathfinder
> is that members of the searchdirs list reserve space for the maxlen
> of basenames:  This implies that the basenames list must not change
> after the first searchdir was registered.
>
> To make sure this doesn't happen I prefer to not provide such an API
> at all, rather than to check within some pathfinder::add_basename ()
> method and abort if there is some searchdir registered already.
Yes, that sounds good.

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


Thanks,
Corinna

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

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 3/4] dlopen: on x/lib search x/bin if exe is in x/bin

Corinna Vinschen-2
In reply to this post by Michael Haubenwallner-2
On Sep  2 10:46, Michael Haubenwallner wrote:
> Hi Corinna,
>
> On 09/01/2016 03:32 PM, Corinna Vinschen wrote:
> > You could just use the global variable program_invocation_name.  If in
> > doubt, use the Windows path global_progname and convert it to full POSIX
> > via cygwin_conv_path.
>
> Patch updated, using global_progname now.

Looks good and you're right to do it this way since I just noticed
that program_invocation_name may return a relative pathname.

Btw., in other calls which require the full POSIX path we use
mount_table->conv_to_posix_path instead of cygwin_conv_path (see
e. g. fillout_pinfo()).  It's a bit faster.  Maybe something for a
followup patch.

Note for some later improvement:  I really wonder why we don't store
the absolute POSIX path of the current executable globally yet...


Thanks,
Cornina

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

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 4/4] dlopen: on unspecified lib dir search exe dir

Michael Haubenwallner-2
In reply to this post by Corinna Vinschen-2
Hi Corinna,

On 09/01/2016 04:05 PM, Corinna Vinschen wrote:

> On Aug 31 20:07, Michael Haubenwallner wrote:
>> Applications installed to some prefix like /opt/application do expect
>> dlopen("libAPP.so") to load "/opt/application/bin/cygAPP.dll", which
>> is similar to "/opt/application/lib/libAPP.so" on Linux.
>>
>> See also https://cygwin.com/ml/cygwin-developers/2016-08/msg00020.html
>>
>> * dlfcn.cc (dlopen): For dlopen("N"), search directory where the
>> application executable is in.
>> ---
>>  winsup/cygwin/dlfcn.cc | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/winsup/cygwin/dlfcn.cc b/winsup/cygwin/dlfcn.cc
>> index f8b8743..974092e 100644
>> --- a/winsup/cygwin/dlfcn.cc
>> +++ b/winsup/cygwin/dlfcn.cc
>> @@ -232,6 +232,12 @@ dlopen (const char *name, int flags)
>>       not use the LD_LIBRARY_PATH environment variable. */
>>    finder.add_envsearchpath ("LD_LIBRARY_PATH");
>>  
>> +  /* Search the current executable's directory like
>> +     the Windows loader does for linked dlls. */
>> +  int exedirlen = get_exedir (cpath, wpath);
>> +  if (exedirlen)
>> +    finder.add_searchdir (cpath, exedirlen);
>> +
>>    /* Finally we better have some fallback. */
>>    finder.add_searchdir ("/usr/bin", 8);
>>    finder.add_searchdir ("/usr/lib", 8);
>> --
>> 2.7.3
>
> Still not quite sure if that's the right thing to do...
Hmm... dlopen ought to be an API to the "runtime loader",
and as such it ought to use the same search algorithm as
exec (=CreateProcess) when searching for the linked dlls.

So as far as I understand: The Windows loader uses the main
executable's directory as kinda "embedded runpath" for process
startup, and dlopen is not the right place to change this - even
if LoadLibrary provides such mechanisms (Set/AddDllDirectory).

btw: Patch 4 updated to follow the update of patch 3.

Thanks!
/haubi/

0004-dlopen-on-unspecified-lib-dir-search-exe-dir.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Michael Haubenwallner-2
In reply to this post by Corinna Vinschen-2
On 09/02/2016 10:52 AM, Corinna Vinschen wrote:

> On Sep  2 10:05, Michael Haubenwallner wrote:
>> On 09/01/2016 04:03 PM, Corinna Vinschen wrote:
>>> On Sep  1 13:05, Michael Haubenwallner wrote:
>>>> On 08/31/2016 09:12 PM, Corinna Vinschen wrote:
>>>>> 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.
>>
>> The idea to build the basenames list before constructing pathfinder
>> is that members of the searchdirs list reserve space for the maxlen
>> of basenames:  This implies that the basenames list must not change
>> after the first searchdir was registered.
>>
>> To make sure this doesn't happen I prefer to not provide such an API
>> at all, rather than to check within some pathfinder::add_basename ()
>> method and abort if there is some searchdir registered already.
>
> Yes, that sounds good.
>
>>> Given that basenames is a member,
>>> you can do the allocator stuff completely inside the pathfinder class.
>>
>> 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.

Thanks a lot!
/haubi/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 3/4] dlopen: on x/lib search x/bin if exe is in x/bin

Michael Haubenwallner-2
In reply to this post by Corinna Vinschen-2
On 09/02/2016 11:03 AM, Corinna Vinschen wrote:

> On Sep  2 10:46, Michael Haubenwallner wrote:
>> On 09/01/2016 03:32 PM, Corinna Vinschen wrote:
>>> You could just use the global variable program_invocation_name.  If in
>>> doubt, use the Windows path global_progname and convert it to full POSIX
>>> via cygwin_conv_path.
>>
>> Patch updated, using global_progname now.
>
> Looks good and you're right to do it this way since I just noticed
> that program_invocation_name may return a relative pathname.
Yep.

> Btw., in other calls which require the full POSIX path we use
> mount_table->conv_to_posix_path instead of cygwin_conv_path (see
> e. g. fillout_pinfo()).  It's a bit faster.  Maybe something for a
> followup patch.

No problem - attached.
This renders the original patch 4/4 valid again.

> Note for some later improvement:  I really wonder why we don't store
> the absolute POSIX path of the current executable globally yet...

Same here.

Thanks!
/haubi/


0003-dlopen-on-x-lib-search-x-bin-if-exe-is-in-x-bin.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Corinna Vinschen-2
In reply to this post by Michael Haubenwallner-2
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.  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.


Thanks,
Corinna

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

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Michael Haubenwallner-2
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.

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...

/haubi/
12
Loading...