From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22786 invoked by alias); 6 Jan 2015 10:14:31 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 34129 Received: (qmail 2843 invoked from network); 6 Jan 2015 10:14:17 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=4OQIk19z9nTqQIRto+8nhccjEVyF6BM0nTCC+De/cT8=; b=cssz5ChekFyVhdqcqUP6Kp4a6UAZvKyVWEXcjJqChJiV54TXo9mAh7iJFprTDXc3OD Q4c1Q3YkNAGxbOonBE9kUOc9DBOZT34hOhccKUX7yf8CnAJ9NJm7wpJeW8hxjBq7G67F coOZaz4VdUhKmWnZvPaOf/ukX0TzRXkYVIUVfN8yZHmh1bK4CgjPwo5YK/GN0zV64crJ 83TMp9ozq7kexFYOvGiGXjoT/hYSj82ZcoXRN98oL82a7gEFFlN20A8zJFbCpteXMxN4 iblBBDylaZNRzi8LqMe8afUpXGE83eIuYkFYvgRmYkeCv6DEBuARLMC6SsTRaPPRr8s3 Y2jA== MIME-Version: 1.0 X-Received: by 10.50.80.36 with SMTP id o4mr15277858igx.37.1420539252039; Tue, 06 Jan 2015 02:14:12 -0800 (PST) In-Reply-To: <20150106095449.081ed9a5@pwslap01u.europe.root.pri> References: <1420521949-30483-1-git-send-email-mikachu@gmail.com> <1420521949-30483-14-git-send-email-mikachu@gmail.com> <20150106095449.081ed9a5@pwslap01u.europe.root.pri> Date: Tue, 6 Jan 2015 11:14:12 +0100 Message-ID: Subject: Re: PATCH 13/17: anon funcs: don't leak shf when ctrl-c in () {:} =(sleep 1) From: Mikael Magnusson To: Peter Stephenson Cc: zsh workers Content-Type: text/plain; charset=UTF-8 On Tue, Jan 6, 2015 at 10:54 AM, Peter Stephenson wrote: > On Tue, 6 Jan 2015 06:25:45 +0100 > Mikael Magnusson 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