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

[Petr Spacek]

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.

-- 
Petr^2 Spacek