Hi Stepan, Noah, * Noah Misch wrote on Wed, Mar 15, 2006 at 05:03:57PM CET: > On Wed, Mar 15, 2006 at 12:47:11PM +0100, Stepan Kasal wrote: > > well, the following patch would save 170 bytes. > > A space optimization of this magnitude is irrelevant at the scope of a > configure > script. Let us ignore this virtue.
Agreed. > > I think setting IFS and unsetting CDPATH is ``sanitizing''. > > I agree with this part. Agreed. > > And I think the stack of depth 1 (as_save_IFS) has no practical value; > > using a global default value is IMHO more sincere. > > No other macro in m4sh uses this variable. One would only run into trouble by > making an AS_PATH_WALK the BODY of another AS_PATH_WALK. If preserving IFS > has > any practical value at all, one-level preservation is not so bad. > > configure.ac authors who change IFS should always revert it before calling > Autoconf macros, or any macros they do not control. Nonetheless, this patch > can > only break existing uses. In the absence of other significant benefits, let > us > forgo this change. Agreed. But: Note that the patch actually fixes two(!) latent issues, which are actually orthogonal to the intended change, by moving initializations to different places. And I think they should be fixed, but without changing the rest of the code (as Noah wrote): Before the patch, IFS would be sanitized only after the first path walk (the one to set as_myself) and after searching for a better shell. Why is this a bug? Take an older (d)ash still present on recent systems[1]. It fails to initialize IFS. Which is fine as long as it is unset: Posix demands unset IFS has the semantics as '<space><tab><newline>'. But after the path walk, IFS will be set to empty. So word splitting is disabled then. Since the better-shell-search happened before the IFS sanitization, the $as_candidate_shells values would not get expanded: | for as_shell in $as_candidate_shells $SHELL; do This isn't terribly bad for ash, but for some older shells it may have been. So it's necessary to move the sanitization of IFS to the earlier place. Second, your change to _AC_CANONICAL_SPLIT is more than just a change of names, but a latent bugfix (presumably the one that you worked around with some change implemented soon after you started using $IFS in _AC_CANONICAL_SPLIT, IIRC the 2005-08-24 one). Cheers, Ralf [1] https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=151276
