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

 review needs-fixing

On Sat, Sep 08, 2012 at 08:35:22AM +0000, Xin wrote:
> Xin has proposed merging lp:~hipl-core/hipl/hipv2-dh-ecdh into lp:hipl.
>
> Requested reviews:
> HIPL core team (hipl-core)
>
> For more details, see:
> https://code.launchpad.net/~hipl-core/hipl/hipv2-dh-ecdh/+merge/123405
>
> 8 September 2012
>
> Revise code based on Diego's review comments.
> Sorry for my delay.

You sort of addressed my comments, but I'm really not sure how thorough.
In order to check for simple oversights I searched this email for "llips"
and immediately caught an error I pointed out but you failed to fix.

It seems that all newly-added memory allocations are checked now. This
is good; unfortunately unchecked memory allocations still happen left
and right in HIPL.

This whole review business is rather pointless if the comments are not
addressed. If I can find mistakes in your revised code literally within
seconds then we have serious quality control issues as a project.

Maybe this is just a minor oversight about a minor issue, maybe not.
Bugs tend to appear in clusters, so I'm suspicious. How could we fix
this? Would some sort of checklist or a list of common mistakes to
look out for help?

> --- libhipl/cookie.c 2012-06-03 10:26:55 +0000
> +++ libhipl/cookie.c 2012-09-08 08:34:18 +0000
> @@ -219,41 +218,93 @@
>
> +struct hip_common *hip_get_r1_v2(struct in6_addr *ip_i, struct in6_addr *ip_r,
> + struct in6_addr *our_hit, const int dh_group_id)
> +{
> + /* Create a copy of the found entry */
> + len = hip_get_msg_total_len(&hid->r1_v2[dh_group_id][idx].buf.msg);
> + if (len <= 0) {
> + HIP_ERROR("Invalid r1_v2 entry at %d, %d\n", dh_group_id, idx);
> + return NULL;
> + }
> +
> + if((r1 = hip_msg_alloc()) == NULL) {

if (

Is your uncrustify hook not working?

Diego