On 03/05/2015 02:45 PM, Petr Spacek wrote:
On 26.2.2015 17:01, Martin Basti wrote:
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.
ACK, it works for me.


Pushed to master, ipa-4-1.

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

Reply via email to