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

Hi,

On 09/10/2012 03:19 PM, Diego Biurrun wrote:
> review needs-fixing
> 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.

oops, I read the merge diff line by line before I commit it, but it
seems that I still miss some points.

>
> 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 is not related to this merge right? If we should discuss how to fix
them, I think it is better to put it into another mail thread, or fire a
bug?

>
> 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.

People do make mistakes, if you check how many "llips" I have fixed and
how I fix your other comments, maybe you will feel a sense of relief.

>> --- 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?

Yeah, I turned it off because some of code changes will violate it, but
I should have checked other codes. you have my apology.

--
Best regards,
Xin Gu