Re: get_full_path_of_dll: lookup application directory for a DLL first

[Christopher Faylor <cgf at redhat dot com>]

I'm sorry to have you go to the effort of cleaning this up and
resubmiitting it but on reviewing your patch, I found a few bothersome
things about dlfcn.cc.  It looks to me like dlfcn.cc is a lot more
complicated than it has to be.  It also looks like there were some
inconsistent practices (using small_printf for error messages) and some
outright errors (returning a pointer to a stack-based variable).  I
suspect that some of these problems were put there by me.

I've ripped out most of get_full_path_of_dll.  I don't see any reason
why it couldn't just 1) search LD_LIBRARY_PATH and if that fails 2)
allow LoadLibrary to find the DLL.  That way we won't have to be second
guessing Microsoft.

So, although your patch was perfect and would have fixed a bug, the code
you were modifying was not perfect and needed to be rewritten.  I'm
going to check in a minor rewrite soon.  If you could verify that it
does the right thing, I'd appreciate it.  In my limited tests it seemed
fine.

Thanks for the patch submission and for alerting me to the quality of
this code.  It had obviously suffered some bit rot.

cgf

On Mon, Mar 26, 2001 at 12:48:33AM +0200, Reinhard Nissl wrote:
>Hi,
>
>attached is a patch for get_full_path_of_dll(), to lookup the application
>directory for a DLL first. The documentation of get_full_path_of_dll() says,
>it should behave like LoadLibrary(), but it did never lookup the application
>directory. I'm not shure, if this patch brakes with any cygwin philosophy.
>
>BTW: The patch is against the current cvs sources.
>
>Motivation:
>I recognized this lack of functionality, while playing with cdrtools-1.10a17
>on Win2K and WinNT. cdrecord wants to load wnaspi32.dll and reported an error,
>when the library was not in one of the directories, that were searched by
>get_full_path_of_dll().
>
>As I share cdrtools with other people on our local network, I'd like to put
>wnaspi32.dll into the directory, where cdrecord.exe resides. After my patch,
>cdrecord can be used easily without the need to configure anyones system in a
>certain way (setup environment variables, copy libraries, change current
>directory, etc.).
>
>Bye.
>--
>Dipl.-Inform. (FH) Reinhard Nissl
>mailto:rnissl@gmx.de
>--- dlfcn.cc-orig	Sun Mar 18 03:34:05 2001
>+++ dlfcn.cc	Mon Mar 26 00:32:52 2001
>@@ -102,6 +102,26 @@ get_full_path_of_dll (const char* str)
>   if (isabspath (str))
>     ret = name;
> 
>+  /* Lookup application directory first (LoadLibrary() behaviour). */
>+  if (!ret)
>+    {
>+      /* Get the absolute file name path of the executeable. */
>+      if (GetModuleFileName (NULL, buf, MAX_PATH) == 0)
>+	small_printf ("WARNING: get_full_path_of_dll can't get module file name win32 %E\n");
>+      else
>+	{
>+	  /* To get the application directory, strip off the file name. */
>+	  char *p = strrchr (buf, '\\');
>+	  if (!p)
>+	    small_printf ("WARNING: get_full_path_of_dll can't extract application directory from module file name\n");
>+	  else
>+	    {
>+	      *p = '\0';
>+	      ret = check_access (buf, name);
>+	    }
>+	}
>+    }
>+
>   /* current directory */
>   if (!ret)
>     {

>2001-03-26 Reinhard Nissl <rnissl@gmx.de>
>
>	* dlfcn.cc (get_full_path_of_dll): Lookup application directory for DLL
>	first (LoadLibrary() behaviour).
>


-- 
cgf@cygnus.com                        Red Hat, Inc.
http://sources.redhat.com/            http://www.redhat.com/