Re: [Partial patch] IFS and read builtin

[Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxxx>]

On Sun, Nov 07, 2010 at 04:04:20PM -0600, Jonathan Nieder wrote:
>
> Another puzzle bisecting to f42e443bb.  This one comes from the
> grub-mkconfig script:
> 
>  $ sh -c 'datadir=/usr/share; pkgdatadir=${datadir}/`cat`' 2>&1 | cat -A
>  cat: M-^\^M^F^HM-4^M^F^HM-(^M^F^H: No such file or directory$
>  cat: M-(^M^F^H: No such file or directory$
> 
> Still reproducible with 016b529.  I'll try to find time to look into
> it, but thought you might like to know nevertheless.

Sure enough it was another leak, hopefully this is the last one :)

commit 49a94e2bab1e4f601a9fbdf9615d9e4e0150e412
Author: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Date:   Sun Nov 28 21:09:51 2010 +0800

    [EXPAND] Free IFS state in evalbackcmd
    
    On Sun, Nov 07, 2010 at 04:04:20PM -0600, Jonathan Nieder wrote:
    >
    > Herbert Xu wrote:
    >
    > > commit f42e443bb511ed3224f09b4fcf0772438ebdbbfa
    > > Author: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
    > > Date:   Wed Sep 8 20:07:26 2010 +0800
    > >
    > >     [EXPAND] Fix ifsfirst/ifslastp leak
    >
    > Another puzzle bisecting to f42e443bb.  This one comes from the
    > grub-mkconfig script:
    >
    >  $ sh -c 'datadir=/usr/share; pkgdatadir=${datadir}/`cat`' 2>&1 | cat -A
    >  cat: M-^\^M^F^HM-4^M^F^HM-(^M^F^H: No such file or directory$
    >  cat: M-(^M^F^H: No such file or directory$
    >
    > Still reproducible with 016b529.  I'll try to find time to look into
    > it, but thought you might like to know nevertheless.
    
    This is the symptom of another leak.  In this case evalbackcmd
    occurs in the middle of an expansion (as it should) but the forked
    child never clears the previous IFS state.
    
    This patch adds the missing ifsfree call.
    
    This wasn't as much of a problem as the previously discovered leaks
    since all it means is that the child gets to carry around the parent's
    expansion state and the child is usually short-lived.
    
    Reported-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
    Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

diff --git a/ChangeLog b/ChangeLog
index 13572bf..d0a8304 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -19,6 +19,7 @@
 
 	* Fixed trap/return regression due to SKIPEVAL removal.
 	* Allow the originator of EXERROR to set the exit status.
+	* Free IFS state in evalbackcmd.
 
 2010-10-18  Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
 
diff --git a/src/eval.c b/src/eval.c
index 64aabb1..6e5c43e 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -625,6 +625,7 @@ evalbackcmd(union node *n, struct backcmd *result)
 				dup2(pip[1], 1);
 				close(pip[1]);
 			}
+			ifsfree();
 			evaltreenr(n, EV_EXIT);
 			/* NOTREACHED */
 		}

Thanks,
-- 
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html