[Freeipa-devel] [PATCH 0195] Fix memory leaks in ipapkcs11helper module

[Martin Basti]

On 26/02/15 13:06, Petr Spacek wrote:
> Hello Martin,
>
> thank you for patch! This NACK is only aesthetic :-)
>
> On 25.2.2015 14:21, Martin Basti wrote:
>>       if (!check_return_value(rv, "import_wrapped_key: key unwrapping")) {
>> +        error = 1;
>> +        goto final;
>> +    }
>
> This exact sequence is repeated many times in the code.
>
> I would prefer a C macro like this:
> #define GOTO_FAIL						\
>          do {							\
>                  error = 1;					\
>                  goto final;					\
>          } while(0)
>
> This allows more dense code like:
> if (!test)
> 	GOTO_FAIL;
>
> and does not have the risk of missing error = 1 somewhere.
>
>> +final:
>>       if (pkey != NULL)
>>           EVP_PKEY_free(pkey);
>> +    if (label != NULL) PyMem_Free(label);
>> +    if (error){
>> +        return NULL;
>> +    }
>>       return ret;
>>   }
> Apparently, this is inconsistent with itself.
>
> Please pick one style and use it, e.g.
> if (label != NULL)
> 	PyMem_Free(label)
>
> ... and do not add curly braces when unnecessary.
>
> If you want, we can try running $ indent on current sources and committing
> changes separately so you do not have to make changes like this by hand.
>
Thanks. Updated patch attached.

-- 
Martin Basti

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0195.2-Fix-memory-leaks-in-ipap11helper.patch
Type: text/x-patch
Size: 26983 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150226/e6c58af5/attachment.bin>