[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Wed, 15 Mar 2006 17:30:43 +0100
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
> script. Let us ignore this virtue.
> > I think setting IFS and unsetting CDPATH is ``sanitizing''.
> I agree with this part.
> > 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
> 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
> only break existing uses. In the absence of other significant benefits, let
> 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.
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).