From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Mark H Weaver Newsgroups: gmane.lisp.guile.devel Subject: [PATCH] Don't augment LD_LIBRARY_PATH (was Re: [PATCH] do not augment environment) Date: Wed, 03 Oct 2012 06:31:15 -0400 Message-ID: <87y5jnn8y4.fsf_-_@tines.lan> References: <505CF9FC.9060007@gmail.com> <5069022C.9040600@netris.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: ger.gmane.org 1349260361 30942 80.91.229.3 (3 Oct 2012 10:32:41 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Wed, 3 Oct 2012 10:32:41 +0000 (UTC) Cc: Bruce Korb To: Guile Development Original-X-From: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Wed Oct 03 12:32:44 2012 Return-path: Envelope-to: guile-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1TJMFI-0008DG-0h for guile-devel@m.gmane.org; Wed, 03 Oct 2012 12:32:04 +0200 Original-Received: from localhost ([::1]:60612 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TJMFC-0004e7-7u for guile-devel@m.gmane.org; Wed, 03 Oct 2012 06:31:58 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:35328) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TJMF3-0004e1-Lw for guile-devel@gnu.org; Wed, 03 Oct 2012 06:31:55 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TJMEx-0004P3-9H for guile-devel@gnu.org; Wed, 03 Oct 2012 06:31:49 -0400 Original-Received: from world.peace.net ([96.39.62.75]:38271) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TJMEx-0004NR-3z for guile-devel@gnu.org; Wed, 03 Oct 2012 06:31:43 -0400 Original-Received: from 209-6-91-212.c3-0.smr-ubr1.sbo-smr.ma.cable.rcn.com ([209.6.91.212] helo=tines.lan) by world.peace.net with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.72) (envelope-from ) id 1TJMEc-00044U-In; Wed, 03 Oct 2012 06:31:23 -0400 In-Reply-To: <5069022C.9040600@netris.org> (Mark H. Weaver's message of "Sun, 30 Sep 2012 22:38:36 -0400") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 96.39.62.75 X-BeenThere: guile-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Developers list for Guile, the GNU extensibility library" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Original-Sender: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.lisp.guile.devel:14925 Archived-At: --=-=-= Content-Type: text/plain Hello all, Here's a preliminary patch to avoid modifying LD_LIBRARY_PATH. Following Bruce's suggestion, it causes 'sysdep_dynl_link' to manually search additional directories if 'lt_dlopenext' fails to find the library in the default paths. However, I took a somewhat different approach, and tried to be more careful with regard to portability and correctness. I read the code of libltdl, and mimicked their handling of path separators and directory separators, to ensure that this patch will not reduce our portability. I also followed their lead in deciding when to perform a search. If any directory separators are present (even if it's a relative pathname), then libltdl does not perform a search. I used precisely the same criterion to decide whether to search additional directories. So what additional directories does it search? If GUILE_SYSTEM_EXTENSIONS_PATH is set (even if it's empty), then it specifies the additional directories to search. If it's unset, then the default is to search SCM_LIB_DIR and SCM_EXTENSIONS_DIR. *** Note that this changes the search order in the case where GUILE_SYSTEM_EXTENSIONS_PATH is set to a non-empty string. Currently, a non-empty GUILE_SYSTEM_EXTENSIONS_PATH is passed to lt_dladdsearchdir, so it is searched before LTDL_LIBRARY_PATH and LD_LIBRARY_PATH, but this patch causes GUILE_SYSTEM_EXTENSIONS_PATH to be searched last, to be consistent with the handling of the default directories SCM_LIB_DIR and SCM_EXTENSIONS_DIR. This seems sensible to me. Does anyone see a problem with this change? This patch also adds robust handling of the case where GUILE_SYSTEM_EXTENSIONS_PATH contains more than one path component. See below for my preliminary patch. I have not yet tested it carefully. It's a context diff, because 'diff' made a mess of the unified diff. Comments and suggestions solicited. Mark --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=DONT_AUGMENT_LD_LIBRARY_PATH.patch Content-Description: Do not augment LD_LIBRARY_PATH v1 diff --git a/libguile/dynl.c b/libguile/dynl.c index a2ae6e2..149ed26 100644 *** a/libguile/dynl.c --- b/libguile/dynl.c *************** *** 26,31 **** --- 26,33 ---- #endif #include + #include + #include /* "dynl.c" dynamically link&load object files. Author: Aubrey Jaffer *************** *** 37,43 **** solution would probably be a shared libgcc. */ #undef NDEBUG - #include static void maybe_drag_in_eprintf () --- 39,44 ---- *************** *** 75,92 **** */ /* njrev: not threadsafe, protection needed as described above */ static void * sysdep_dynl_link (const char *fname, const char *subr) { lt_dlhandle handle; ! if (fname != NULL) ! handle = lt_dlopenext (fname); else ! /* Return a handle for the program as a whole. */ ! handle = lt_dlopen (NULL); ! if (NULL == handle) { SCM fn; SCM msg; --- 76,165 ---- */ /* njrev: not threadsafe, protection needed as described above */ + + /* 'system_extensions_path' is used by 'sysdep_dynl_link' to search for + dynamic libraries as a last resort, when they cannot be found in the + usual library search paths. */ + static char *system_extensions_path; + static void * sysdep_dynl_link (const char *fname, const char *subr) { lt_dlhandle handle; ! if (fname == NULL) ! { ! /* Return a handle for the program as a whole. */ ! handle = lt_dlopen (NULL); ! } else ! { ! handle = lt_dlopenext (fname); ! ! if (handle == NULL ! #ifdef LT_DIRSEP_CHAR ! && strchr (fname, LT_DIRSEP_CHAR) == NULL ! #endif ! && strchr (fname, '/') == NULL) ! { ! /* 'fname' contains no directory separators and was not in the ! usual library search paths, so now we search for it in the ! directories specified in 'system_extensions_path'. */ ! char *fname_attempt = malloc (strlen (system_extensions_path) ! + strlen (fname) ! + 1 /* for directory separator */ ! + 1); /* for null terminator */ ! char *path; /* remaining path to search */ ! char *end; /* end of current path component */ ! char *s; ! ! if (fname_attempt != NULL) ! { ! scm_dynwind_begin (0); ! scm_dynwind_free (fname_attempt); ! ! /* Iterate over the components of 'system_extensions_path' */ ! for (path = system_extensions_path; ! *path != '\0'; ! path = (*end == '\0') ? end : (end + 1)) ! { ! /* Find end of pathname component */ ! end = strchr (path, LT_PATHSEP_CHAR); ! if (end == NULL) ! end = strchr (path, '\0'); ! ! /* Skip empty path components */ ! if (path == end) ! continue; ! ! /* Construct 'fname_attempt', starting with path component */ ! s = fname_attempt; ! memcpy (s, path, end - path); ! s += end - path; ! ! /* Append directory separator, but avoid duplicates */ ! if (s[-1] != '/' ! #ifdef LT_DIRSEP_CHAR ! && s[-1] != LT_DIRSEP_CHAR ! #endif ! ) ! *s++ = '/'; ! ! /* Finally, append 'fname' with null terminator */ ! strcpy (s, fname); ! ! /* Try to load it, and terminate the search if successful */ ! handle = lt_dlopenext (fname_attempt); ! if (handle != NULL) ! break; ! } ! ! scm_dynwind_end (); ! } ! } ! } ! if (handle == NULL) { SCM fn; SCM msg; *************** *** 120,149 **** return fptr; } - /* Augment environment variable VARIABLE with VALUE, assuming VARIABLE - is a path kind of variable. */ - static void - augment_env (const char *variable, const char *value) - { - const char *env; - - env = getenv (variable); - if (env != NULL) - { - char *new_value; - static const char path_sep[] = { LT_PATHSEP_CHAR, 0 }; - - new_value = alloca (strlen (env) + strlen (value) + 2); - strcpy (new_value, env); - strcat (new_value, path_sep); - strcat (new_value, value); - - setenv (variable, new_value, 1); - } - else - setenv (variable, value, 1); - } - static void sysdep_dynl_init () { --- 193,198 ---- *************** *** 151,176 **** lt_dlinit (); env = getenv ("GUILE_SYSTEM_EXTENSIONS_PATH"); ! if (env && strcmp (env, "") == 0) ! /* special-case interpret system-ltdl-path=="" as meaning no system path, ! which is the case during the build */ ! ; ! else if (env) ! /* FIXME: should this be a colon-separated path? Or is the only point to ! allow the build system to turn off the installed extensions path? */ ! lt_dladdsearchdir (env); else { ! /* Add SCM_LIB_DIR and SCM_EXTENSIONS_DIR to the loader's search ! path. `lt_dladdsearchdir' and $LTDL_LIBRARY_PATH can't be used ! for that because they are searched before the system-dependent ! search path, which is the one `libtool --mode=execute -dlopen' ! fiddles with (info "(libtool) Libltdl Interface"). See ! ! for details. */ ! augment_env (SHARED_LIBRARY_PATH_VARIABLE, SCM_LIB_DIR); ! augment_env (SHARED_LIBRARY_PATH_VARIABLE, SCM_EXTENSIONS_DIR); } } --- 200,232 ---- lt_dlinit (); + /* Initialize 'system_extensions_path' from + $GUILE_SYSTEM_EXTENSIONS_PATH, or if that's not set: + . + + 'lt_dladdsearchdir' can't be used because it is searched before the + system-dependent search path, which is the one 'libtool + --mode=execute -dlopen' fiddles with (info "(libtool) Libltdl + Interface"). See + . + + The environment variables $LTDL_LIBRARY_PATH and $LD_LIBRARY_PATH + can't be used because they would be propagated to subprocesses + which may cause problems for other programs. See + */ + env = getenv ("GUILE_SYSTEM_EXTENSIONS_PATH"); ! if (env) ! system_extensions_path = env; else { ! system_extensions_path = (char *) malloc (strlen (SCM_LIB_DIR) ! + strlen (SCM_EXTENSIONS_DIR) ! + 1 /* for path separator */ ! + 1); /* for null terminator */ ! assert (system_extensions_path != NULL); ! sprintf (system_extensions_path, "%s%c%s", ! SCM_LIB_DIR, LT_PATHSEP_CHAR, SCM_EXTENSIONS_DIR); } } --=-=-=--