Code review comment for lp:~rick-fdd/pyopenssl/pkcs12_mod_and_export2

Revision history for this message
rick_dean (rick-fdd) wrote :

My previous email was sent too early.

#149 I'm fine without a warning.

#156 Great catch. I hadn't noticed that PKCS12_parse()
would free the CA list, but not NULL it. For shame!

#157 Okay. Error paths are so hard to check.
A friend in a large company (that incidentally
uses a *lot* of python) says the culture there
is to always call exit() on errors. You can
log a message first if you like, but don't bother
wasting code on an error path which is futile
to get right.

--
Rick

On Sun, Jul 26, 2009 at 01:57:57AM -0000, Jean-Paul Calderone wrote:
> >
> > Excellent points! Thanks for the lesson. I pushed all five
> > fixes to lp:~rick-fdd/pyopenssl/pkcs12_mod_and_export2
> >
>
> Awesome, thanks for all your work on this. :) I made a bunch of cosmetic changes (I should put
> together a style guide for pyOpenSSL, but I'm still not sure what it would say), but notices a
> couple more actual potential issues, which I also tried to tackle. My changes are in a branch
> based on yours, <lp:~exarkun/pyopenssl/pkcs12_mod_and_export2>. The revisions which actually
> contain interesting changes are:
>
> http://bazaar.launchpad.net/~exarkun/pyopenssl/pkcs12_mod_and_export2/revision/149
> http://bazaar.launchpad.net/~exarkun/pyopenssl/pkcs12_mod_and_export2/revision/156
> http://bazaar.launchpad.net/~exarkun/pyopenssl/pkcs12_mod_and_export2/revision/157
>
> I think I'm happy with the code now, so if you think these changes are sensible, I think I'll go ahead and merge them into trunk.
>
> Jean-Paul
>
> --
> https://code.launchpad.net/~rick-fdd/pyopenssl/pkcs12_mod_and_export2/+merge/8962
> You are the owner of lp:~rick-fdd/pyopenssl/pkcs12_mod_and_export2.

--
Rick

« Back to merge proposal