Re: [PATCH] trap: fix memory leak in exitshell()

["Andreas Bofjäll" <andreas@gazonk.org>]

On Tue, 22 Nov 2016, Jilles Tjoelker wrote:

> On Mon, Nov 21, 2016 at 10:40:52PM +0100, Andreas Bofjall wrote:
>> diff --git a/src/trap.c b/src/trap.c
>> index edb9938..5418b07 100644
>> --- a/src/trap.c
>> +++ b/src/trap.c
>> @@ -389,6 +389,7 @@ exitshell(void)
>>  		trap[0] = NULL;
>>  		evalskip = 0;
>>  		evalstring(p, 0);
>> +		ckfree(p);
>>  	}
>>  out:
>>  	/*
>
> This patch will shut up valgrind in the common case, but does not handle
> the general case. The command string may contain an error or invoke the
> exit builtin and in either case the command string will be leaked
> (SIGINT might be expected to have a similar effect, but behaves
> strangely from an EXIT trap in dash).
>
> You can probably use the exception handling already present in the
> function to fix this. Note that ckfree() should only be used while
> INTOFF is in effect, both to avoid longjmp'ing out of free() and to
> ensure exactly one free in the presence of interruptions and errors.

Thanks for the feedback! Would something simple like moving the call to 
ckfree() to after the out: label and wrapping it in INTOFF/INTON (thereby 
catching both the normal return path and the exception) be ok, or do you 
mean something more elaborate?

/Andreas