[PATCH 1/6] sexp: fix invalid deallocation in error path

[Peter Wu]

Hi,

On Fri, Jul 10, 2015 at 11:11:08AM +0900, NIIBE Yutaka wrote:
> Thank you for the fix.  I'd like to commit your fix without your
> comment since it is not accurate.  Is it OK for you?

You are right, I swapped the case. Feel free to adjust it to whatever
you deem appropriate.

> On 07/10/2015 12:11 AM, Peter Wu wrote:
> > --
> > This appears to be a copy and paste error and could result in wrong
> > memory being freed in the error path (when arrayisdesc[idx] == 2, the
> > condition ((!2) == 1) is false).
> 
> 
> When arrayisdesc[idx] == 1, it means that the buffer is provided by
> caller.
> 
> When arrayisdesc[idx] == 2, it means that the buffer is allocated here
> in _gcry_sexp_vextract_param.
> 
> When arrayisdesc[idx] == 1, the wrong expression '!arrayisdesc[idx] == 1'
> is evaluated to !1 == 1 -> 0 == 1 -> 0 (false) and it doesn't go into
> the statements to set spec->len = 0 but goes into 'else' clause to
> free the buffer.   THIS IS A PROBLEM.

Right, thanks for laying this down.

> When arrayisdesc[idx] == 2, the wrong expression '!arrayisdesc[idx] == 1'
> is evaluated to !2 == 1 -> 0 == 1 -> 0 (false) and it goes into 'else'
> clause (which is correct behaviour).

By accident, yes. This patch corrects the 1 case.

Kind regards,
Peter