Re: PATCH 14/17: getsubsargs: free ptr1 before returning

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Tue, 6 Jan 2015 06:25:46 +0100
Mikael Magnusson <mikachu@xxxxxxxxx> 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