Hi,

some time ago, I've reported memory leak that occurs with aliases. This leak was fixed, but it still occured when there was no space after alias.

For example:

alias lsl='ls -l'
a=`lsl `
b=`lsl`

where "a" did not leak, but "b" did. This was fixed by simple patch, that added ' ' to the end of command substitution:
--- sh/macro.c.heresub       2014-05-21 16:48:42.650700910 +0200
+++ sh/macro.c       2014-05-21 16:48:42.678700772 +0200
@@ -2085,6 +2085,7 @@ static void comsubst(Mac_t *mp,register
                        }
                        sfputc(stkp,c);
                }
+               sfputc(stkp,' ');
                c = stktell(stkp);
                str=stkfreeze(stkp,1);
                /* disable verbose and don't save in history file */

but this fix caused a problem later with here documents, so it was changed to sfputc(stkp,'\n'); but another problem occurred - with missing ending quote. The parser tries to fix missing quote by adding one at the end of the input. Which means that originally it would change "abc to "abc" but with the \n it changes "abc to "abc\n" This causes obfuscated error message. While syntax error message would be ok, the error message now is file not found.

Reproducer:
$ touch abc
$ echo `ls "abc`
ls: abc
: not found

This leads to user confusion.


I was looking at the original memory leak and found that it is caused by setupalias:

valgrind: 528 bytes in 3 blocks are still reachable in loss record 106 of 125
malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
sfnew (sfnew.c:97)
_sfopen (_sfopen.c:94)
setupalias (lex.c:2518)
sh_lex (lex.c:1541)
skipnl (parse.c:1654)
term (parse.c:564)
UnknownInlinedFun (parse.c:547)
sh_cmd (parse.c:497)
sh_parse (parse.c:386)
comsubst (macro.c:2099)
copyto (macro.c:600)
sh_mactrim (macro.c:183)
nv_setlist (name.c:339)

When input is read whole, fcfile set to zero and later in setupalias it gets reinitialized with new stream:
# if(!(base=fcfile()))
#     base = sfopen(NIL(Sfio_t*),fcseek(0),"s");
# fcclose();
# sfstack(base,iop);
# fcfopen(base);
but this new stream is never freed.

It seems to me that in sh_lex, the old stream is always available, it just can be "forgotten"

setupalias happens either:
a) when stream is still linked in fcfile and in that case the setupalias won't open new one
or
b) input was read all and sh_lex S_EOF removed the link, but it also saved it in 'sp' variable.

I've tried to use this pointer later, if fcfile is zero and it fixed the leak and did not cause any regression.

--- sh/lex.c.patched   2015-04-17 16:44:46.591326703 +0200
+++ sh/lex.c 2015-04-17 17:06:40.421379298 +0200
@@ -327,7 +327,7 @@ int sh_lex(Lex_t* lp)
        Stk_t                   *stkp = shp->stk;
        int             inlevel=lp->lexd.level, assignment=0, ingrave=0;
        int             epatchar=0;
-       Sfio_t *sp;
+       Sfio_t *sp=NULL;
 #if SHOPT_MULTIBYTE
        LEN=1;
 #endif /* SHOPT_MULTIBYTE */
@@ -1538,6 +1538,8 @@ breakloop:
 #endif /* KSHELL */
                                && (state=nv_getval(np)))
                        {
+                               if (!fcfile())
+                                       _Fcin._fcfile = sp;
                                setupalias(lp,state,np);
                                nv_onattr(np,NV_NOEXPAND);
                                lp->lex.reservok = 1;


Michal
_______________________________________________
ast-developers mailing list
[email protected]
http://lists.research.att.com/mailman/listinfo/ast-developers

Reply via email to