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

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to