Code review comment for lp:~hipl-core/hipl/hipv2-dh-ecdh

 review needs-info

On Thu, Nov 01, 2012 at 03:16:20PM +0000, Xin wrote:
> On 10/30/2012 03:52 AM, Diego Biurrun wrote:
> > [...]
> >
> For these two comments, I don't think they are so similar that worth
> merging together. They have similar logic but operate on different data
> structures, also they are quite short.

We lack proper abstraction for the different crypto primitives.

> All other comments should be fixed. See
> http://bazaar.launchpad.net/~hipl-core/hipl/hipv2-dh-ecdh/revision/6269?remember=6269&compare_revid=6265
> <http://bazaar.launchpad.net/%7Ehipl-core/hipl/hipv2-dh-ecdh/revision/6269?remember=6269&compare_revid=6265>
> for my changes. Thanks.

This is looking much better now, thanks. Did you run your branch through
valgrind to check for memory handling errors? Also, did you try runnning
the unit tests through valgrind. That's a good way to spot errors, also,
the unit tests should be valgrind clean so that each new warning that
appears in the future is a clear bug and not a false positive.

Diego