From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <zsh-workers-return-34127-mason-zsh=primenet.com.au@zsh.org>
Received: (qmail 23227 invoked by alias); 6 Jan 2015 10:03:25 -0000
Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm
Precedence: bulk
X-No-Archive: yes
List-Id: Zsh Workers List <zsh-workers.zsh.org>
List-Post: <mailto:zsh-workers@zsh.org>
List-Help: <mailto:zsh-workers-help@zsh.org>
X-Seq: 34127
Received: (qmail 24753 invoked from network); 6 Jan 2015 10:03:24 -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=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI,
	SPF_HELO_PASS autolearn=ham version=3.3.2
X-AuditID: cbfec7f5-b7fc86d0000066b7-f2-54abb2e94b65
Date: Tue, 06 Jan 2015 10:03:00 +0000
From: Peter Stephenson <p.stephenson@samsung.com>
To: zsh-workers@zsh.org
Subject: Re: PATCH 14/17: getsubsargs: free ptr1 before returning
Message-id: <20150106100300.565ccdea@pwslap01u.europe.root.pri>
In-reply-to: <1420521949-30483-15-git-send-email-mikachu@gmail.com>
References: <1420521949-30483-1-git-send-email-mikachu@gmail.com>
 <1420521949-30483-15-git-send-email-mikachu@gmail.com>
Organization: Samsung Cambridge Solution Centre
X-Mailer: Claws Mail 3.7.9 (GTK+ 2.22.0; i386-redhat-linux-gnu)
MIME-version: 1.0
Content-type: text/plain; charset=US-ASCII
Content-transfer-encoding: 7bit
X-Brightmail-Tracker:
 H4sIAAAAAAAAA+NgFlrCLMWRmVeSWpSXmKPExsVy+t/xy7ovN60OMbjQYWVxsPkhkwOjx6qD
	H5gCGKO4bFJSczLLUov07RK4MqaeXsRasJ+jYsmntywNjNvZuhg5OCQETCQmLbToYuQEMsUk
	LtxbDxTm4hASWMooce7sRyhnCZPEvtffWSGcbYwS7a/PsYO0sAioSixe2MgKYrMJGEpM3TSb
	EcQWERCXOLv2PAvIBmEBR4kvx8VAwrwC9hLvz3xmAbE5BZwlPn5YATZGSKBK4s77tWA2v4C+
	xNW/n5ggLrKXmHnlDCNEr6DEj8n3wHqZBbQkNm9rYoWw5SU2r3nLDDFHXeLG3d3sExiFZiFp
	mYWkZRaSlgWMzKsYRVNLkwuKk9JzjfSKE3OLS/PS9ZLzczcxQkL26w7GpcesDjEKcDAq8fB6
	dK0OEWJNLCuuzD3EKMHBrCTCu2M6UIg3JbGyKrUoP76oNCe1+BAjEwenVAOj3aqfy5JFXXkO
	1Sn3HDySGxs9adGBKmbDG1qrXrGwM9wx+LT1R4n31mtn4/Qi7vmJBJUaNd/mZm0Wivigezs1
	bLVAXvPmNfaLnq9SCJi4TO3NOl1f6fd5PyeYPb98527fBFuRuXcvd5oXdqXunzfniNrnjJ2a
	qu5r/ePPt1qZFIo+v1YkGSqlxFKckWioxVxUnAgAMFTJdzcCAAA=

On Tue, 6 Jan 2015 06:25:46 +0100
Mikael Magnusson <mikachu@gmail.com> wrote:
> Found by Coverity (Issue 439073).
> ---
>  Src/hist.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Src/hist.c b/Src/hist.c
> index d8192c1..e65d78b 100644
> --- a/Src/hist.c
> +++ b/Src/hist.c
> @@ -367,6 +367,7 @@ getsubsargs(char *subline, int *gbalp, int *cflagp)
>  	zsfree(hsubl);
>  	hsubl = ptr1;
>      } else if (!hsubl) {		/* fail silently on this */
> +	zsfree(ptr1);
>  	zsfree(ptr2);
>  	return 0;
>      }

Looks fine.

Separately, it's a little odd we test !ptr1 a few lines above but not
!ptr2.  I don't see any sign that hsubr = NULL is valid.  We might want

    if (!ptr2) {
        zsfree(ptr1);
	return 1;
    }

However, it looks like that can only be a memory failure from
hdynread2() that the function makes no attempt to handle internally.
There are a zillion other places where we have no idea how to fail
gracefully even with much larger chunks of memory, so the existing test
for !ptr1 doesn't really make sense on its own.

pws