Attaching updated version of the module-loading patch. This makes (maybe partially) the git m4 usable in our RPM distros (just testing purposes): https://copr.fedorainfracloud.org/coprs/praiskup/autotools/build/343338/
Pavel On Tuesday, November 25, 2014 3:49:10 PM CEST Gary V. Vaughan wrote: > Hi Pavel, > > > On Nov 22, 2014, at 3:57 PM, Pavel Raiskup <[email protected]> wrote: > > > > 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. > > Yes, indeed. I'll consider both in parallel before I decide what to commit, > thanks for pointing out the similarities in intent. > > > 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'. > > The idea is to minimize the proliferation of additional language keywords > in the GNU version, because each new builtin affects the behaviour of > macro expansion compared to a non-GNU make when a matching bareword is > encountered. Arguably only in pathological cases, but even so, I'd like > to avoid additional builtins where it can be done elegantly. > > I'm inclined to agree that conflating m4 text modules with loadable modules > is a mistake, and M4_EXT_PATH is a good solution - especially as it might > be desirable to put binary modules under /usr/local/lib and text modules > under /usr/local/share (for example). > > I'll try to commit some improvements in this direction over the coming days. > Thanks for the pointer! > > > * Is it desired that $PWD is used for module searching? That seems to be > > rather security hole. > > Certainly for POSIX compatibility with text modules, we must support loading > from the current directory. But, I agree that one can do rather more harm > with a binary loadable module, so it's definitely a win to separate out the > search paths for each. > > > * Don't we want rather load only modules having concrete platform-default > > file extension (e.g. '*.so*' files on Linux)? > > Yes, this is an artifact of unfinished removal of reliance on libltdl. I'll > try to commit some improvements in this area over the coming days too. > > > For the beginning, would anyone be willing to review/push patches for > > that? > > Subject to my comments above, most definitely! And it may be a few days > before I get to it, so please feel free to beat me to the punch if you > have time :-)
>From e3e39c3aacd23802a43696f0bc6fdff0d7574612 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup <[email protected]> Date: Thu, 16 Jun 2016 13:42:13 +0200 Subject: [PATCH] modules, inclusions: fix path searching issues When 'm4' directory occurred in M4PATH or current directory (happens very often with autotooled packages), 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): De-duplicate 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. * tests/others.at: As we now return really the _first_ errno occured during path searching, we need to adjust expected error output. * doc/m4.texi: likewise. --- doc/m4.texi | 2 + m4/path.c | 126 ++++++++++++++++++++++++++++------------------------- tests/options.at | 6 ++- tests/others.at | 8 ++-- tests/testsuite.at | 1 + 5 files changed, 77 insertions(+), 66 deletions(-) diff --git a/doc/m4.texi b/doc/m4.texi index ba1b157..7f0d375 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 ef743a3..a1ff090 100644 --- a/m4/path.c +++ b/m4/path.c @@ -182,6 +182,63 @@ 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 = xrealloc (filepath, mem + max_suffix_len + 1); + +#if TRUNCATE_FILENAME + filepath = path_truncate (filepath); + mem = strlen (filepath); /* recalculate length after truncation */ +#endif + +#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 @@ -220,69 +277,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 = strncpy (xmalloc (mem + max_suffix_len +1), filename, mem +1); -#if TRUNCATE_FILENAME - filepath = path_truncate (filepath); - mem = strlen (filepath); /* recalculate length after truncation */ -#endif - 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); + return try_suffixes (context, NULL, filename, max_suffix_len, suffixes); - /* No such file. */ - errno = e; - return NULL; - } - - 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; + char *pathname = try_suffixes (context, incl->dir, filename, + max_suffix_len, suffixes); + if (pathname) + return pathname; - filepath = strncpy (xmalloc (mem + max_suffix_len +1), pathname, mem +1); - free (pathname); -#if TRUNCATE_FILENAME - filepath = path_truncate (filepath); - mem = strlen (filepath); /* recalculate length after truncation */ -#endif - - 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/others.at b/tests/others.at index 9c7a0c6..507ef78 100644 --- a/tests/others.at +++ b/tests/others.at @@ -144,15 +144,15 @@ AT_DATA([in3.m4], AT_CHECK_M4([in1.m4/], [1], [], [stderr]) dnl mingw fails with EINVAL rather than the expected ENOTDIR -AT_CHECK([$SED 's/Invalid argument/Not a directory/' stderr], [0], -[[m4: cannot open file 'in1.m4/': Not a directory +AT_CHECK([$SED 's/Invalid argument/No such file or directory/' stderr], [0], +[[m4: cannot open file 'in1.m4/': No such file or directory ]]) AT_CHECK_M4([in1.m4], [1], [[ ]], [stderr]) dnl mingw fails with EINVAL rather than the expected ENOTDIR -AT_CHECK([$SED 's/Invalid argument/Not a directory/' stderr], [0], -[[m4:in1.m4:1: include: cannot open file 'in2.m4/': Not a directory +AT_CHECK([$SED 's/Invalid argument/No such file or directory/' stderr], [0], +[[m4:in1.m4:1: include: cannot open file 'in2.m4/': No such file or directory ]]) AT_CHECK_M4([in2.m4], [0], [[ diff --git a/tests/testsuite.at b/tests/testsuite.at index a7cdfc4..20246e2 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])]) ]) -- 2.7.4
_______________________________________________ M4-patches mailing list [email protected] https://lists.gnu.org/mailman/listinfo/m4-patches
