Re: PATCH 13/17: anon funcs: don't leak shf when ctrl-c in () {:} =(sleep 1)

[Mikael Magnusson <mikachu@xxxxxxxxx>]

On Tue, Jan 6, 2015 at 10:54 AM, Peter Stephenson
<p.stephenson@xxxxxxxxxxx> wrote:
> On Tue, 6 Jan 2015 06:25:45 +0100
> Mikael Magnusson <mikachu@xxxxxxxxx> wrote:
>> Found by Coverity (Issue 439076).
>> ---
>>  Src/exec.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/Src/exec.c b/Src/exec.c
>> index 6b93008..207e8c1 100644
>> --- a/Src/exec.c
>> +++ b/Src/exec.c
>> @@ -4461,6 +4461,8 @@ execfuncdef(Estate state, Eprog redir_prog)
>>           if (htok && args) {
>>               execsubst(args);
>>               if (errflag) {
>> +                 zsfree(shf->filename);
>> +                 zfree(shf, sizeof(*shf));
>>                   state->pc = end;
>>                   return 1;
>>               }
>
> Can't see how that can be wrong.  Nothing else can take owernship of
> shf in the error case.

I think what I was unsure of in this one, but forgot to write, is if
freeeprog(shf->funcdef) should also be called here? It looks like for
!names, it's all allocated on the heap, but then the non-error path
does free it explicitly anyway (and it looked like freeeprog just
doesn't touch it if it was on the heap), is that just paranoia? And
likewise for shf->redir. And since we have all these heap vs malloc
checks already, why not use zhalloc for shf too? Although the code
might end up neater if we just always do the malloc + free regardless
since there's a bunch of almost-identical code for that stuff.

-- 
Mikael Magnusson