On Friday 21 of November 2014 09:49:45 Pavel Raiskup wrote: > 0001-build-fix-bootstrap-fail.patch
This is OK now, thanks for fixing it in HEAD. > 0002-modules-inclusions-fix-path-searching-issues.patch Here I tried to reformat once more the patch against master. Result is attached as 0001-modules-inclusions-fix-path-searching-issues.patch. That patch is to make the modules work at all for me. However, thinking about it over and over again, there is something wrong. FWIW, the KO Myung-Hun's patch [1] seems to deal with the same. When I run ./src/m4, it fails - because, by loading of required 'm4' module, 'm4' binary search the $PWD directory. Because it also searches for modules without suffix, directory 'm4' is found and dlopen() is performed on it. * Do we really want M4PATH (thus -I) paths use for module loading? It seems to me that something like M4_EXT_PATH is needed and we should not mix 'include' with 'load'. * Is it desired that $PWD is used for module searching? That seems to be rather security hole. * Don't we want rather load only modules having concrete platform-default file extension (e.g. '*.so*' files on Linux)? For the beginning, would anyone be willing to review/push patches for that? [1] http://www.mail-archive.com/[email protected]/msg01116.html Pavel
>From 30ae36ede39c354673f75c26cf45d9379fff62d3 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup <[email protected]> Date: Wed, 28 May 2014 12:25:52 +0200 Subject: [PATCH] modules, inclusions: fix path searching issues When 'm4' directory occurred in M4PATH or current directory, the m4 processor ended up with message 'Is a directory' and did not continue to search for 'm4.so' somewhere else in path. Expanding 'include(Makefile)' from $(srcdir) for example failed when M4PATH was set because the current directory was not searched. * m4/path.c (try_prefixes): New function. (m4_path_search): Deduplicate suffix trying by try_prefixes. (m4__include_init): Always prepend current directory. * tests/testsuite.at (AT_CHECK_M4): Filter out test output dependant on user's setup. * tests/options.at: Adjust expected stderr output. * doc/m4.texi: likewise. --- doc/m4.texi | 2 + m4/path.c | 113 +++++++++++++++++++++++++++++------------------------ tests/options.at | 6 ++- tests/testsuite.at | 1 + 4 files changed, 68 insertions(+), 54 deletions(-) diff --git a/doc/m4.texi b/doc/m4.texi index e09f3c4..e938a30 100644 --- a/doc/m4.texi +++ b/doc/m4.texi @@ -4683,6 +4683,8 @@ @comment options: -dip @example $ @kbd{m4 -dip -I doc/examples} +@error{}m4debug: path search for 'm4' found 'm4.so' +@error{}m4debug: path search for 'gnu' found 'gnu.so' @error{}m4debug: input read from 'stdin' define(`foo', `m4wrap(`wrapped text ')dnl') diff --git a/m4/path.c b/m4/path.c index 44ed620..130c397 100644 --- a/m4/path.c +++ b/m4/path.c @@ -185,6 +185,58 @@ m4_add_include_directory (m4 *context, const char *dir, bool prepend) #endif } +/* Search for m4 module in DIRNAME directory. Try to test _all_ SUFFIXES with + FILENAME; use the first combination which seems to be correct m4 module. If + the FILENAME is absolute path, DIRNAME shall be NULL. */ +static char * +try_suffixes (m4 *context, const char *dirname, const char *filename, + size_t max_suffix_len, const char **suffixes) +{ + int e = 0, i; + char *filepath = dirname ? file_name_concat (dirname, filename, NULL) + : strdup (filename); + size_t mem = strlen (filepath); + filepath = path_truncate(xrealloc (filepath, mem + max_suffix_len + 1)); + +#ifdef DEBUG_INCL + xfprintf (stderr, "path_search (%s) -- trying %s\n", filename, filepath); +#endif + + /* If search fails, we'll use the errno we got from the first unsuccessful + try. */ + for (i = 0; suffixes && suffixes[i]; !i && (e = errno), ++i) + { + struct stat st; + int rc; + strcpy (filepath + mem, suffixes[i]); + + /* Use stat() here rather than access() because access does not give us + S_ISDIR(st.st_mode) info. Choosing a module which we have no + privileges to open is not problem because dlopening later will fail + anyway with a understandable error. */ + if (stat (filepath, &st)) + continue; + + if (S_ISREG (st.st_mode)) + { + m4_debug_message (context, M4_DEBUG_TRACE_PATH, + _("path search for %s found %s"), + quotearg_style (locale_quoting_style, filename), + quotearg_n_style (1, locale_quoting_style, + filepath)); + return filepath; + } + + if (S_ISDIR (st.st_mode)) + errno = EISDIR; + } + free (filepath); + + /* No such file. */ + errno = e; + return NULL; +} + /* Search for FILENAME according to -B options, `.', -I options, then M4PATH environment. If successful, return the open file, and if @@ -223,61 +275,18 @@ m4_path_search (m4 *context, const char *filename, const char **suffixes) /* If file is absolute, or if we are not searching a path, a single lookup will do the trick. */ if (IS_ABSOLUTE_FILE_NAME (filename)) - { - size_t mem = strlen (filename); - - /* Try appending each of the suffixes we were given. */ - filepath = path_truncate (strncpy (xmalloc (mem + max_suffix_len +1), filename, mem)); - for (i = 0; suffixes && suffixes[i]; ++i) - { - strcpy (filepath + mem, suffixes[i]); - if (access (filepath, R_OK) == 0) - return filepath; - - /* If search fails, we'll use the error we got from the first - access (usually with no suffix). */ - if (i == 0) - e = errno; - } - free (filepath); - - /* No such file. */ - errno = e; - return NULL; - } + return try_suffixes (context, NULL, filename, max_suffix_len, suffixes); - for (incl = m4__get_search_path (context)->list; - incl != NULL; incl = incl->next) + for (incl = m4__get_search_path (context)->list, i = 0; + incl != NULL; incl = incl->next, ++i) { - char *pathname = file_name_concat (incl->dir, filename, NULL); - size_t mem = strlen (pathname); - -#ifdef DEBUG_INCL - xfprintf (stderr, "path_search (%s) -- trying %s\n", filename, pathname); -#endif - - if (access (pathname, R_OK) == 0) - { - m4_debug_message (context, M4_DEBUG_TRACE_PATH, - _("path search for %s found %s"), - quotearg_style (locale_quoting_style, filename), - quotearg_n_style (1, locale_quoting_style, pathname)); - return pathname; - } - else if (!incl->len) - /* Capture errno only when searching `.'. */ - e = errno; - - filepath = path_truncate (strncpy (xmalloc (mem + max_suffix_len +1), pathname, mem)); - free (pathname); + char *pathname = try_suffixes (context, incl->dir, filename, + max_suffix_len, suffixes); + if (pathname) + return pathname; - for (i = 0; suffixes && suffixes[i]; ++i) - { - strcpy (filepath + mem, suffixes[i]); - if (access (filepath, R_OK) == 0) - return filepath; - } - free (filepath); + if (i == 0) + e = errno; } errno = e; diff --git a/tests/options.at b/tests/options.at index 7084207..8fa19b6 100644 --- a/tests/options.at +++ b/tests/options.at @@ -423,10 +423,12 @@ m4debug: input from m4wrap exhausted dnl Test all flags. AT_CHECK_M4([-dV in], [0], [[3 0 -]], [[m4debug: module m4: opening file +]], [[m4debug: path search for 'm4' found 'm4.so' +m4debug: module m4: opening file m4debug: module m4: init hook called m4debug: module m4: opened m4debug: module m4: builtins loaded +m4debug: path search for 'gnu' found 'gnu.so' m4debug: module gnu: opening file m4debug: module gnu: init hook called m4debug: module gnu: opened @@ -701,7 +703,7 @@ AT_CHECK_M4([-I post -B pre in], [1], [[in pre/foo in ./bar in post/blah -]], [[m4:in:3: include: cannot open file 'bad': No such file or directory +]], [[m4:in:3: include: cannot open file 'bad': Too many levels of symbolic links ]]) AT_CLEANUP diff --git a/tests/testsuite.at b/tests/testsuite.at index aad7f0d..5778c8d 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -89,6 +89,7 @@ m4_case([$4], [], [], [ignore], [], /^m4debug: module/s/opening file.*/opening file/ s/\(cannot open module [^:]*\):.*/\1/ s/Bad file number/Bad file descriptor/ + s|search for \([^ ]*\) found '\''.*\/\([^\.]*\.so\)|search for \1 found '\''\2| s/^m4:.* option .*/m4: bad option/ ' stderr >&2]], [0], [], [$4])]) ]) -- 1.9.3
_______________________________________________ M4-patches mailing list [email protected] https://lists.gnu.org/mailman/listinfo/m4-patches
