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

On 15.09.2012 18:46, Xin wrote:
>>> - HIP_ASSERT(item_size == sizeof(uint16_t) || item_size == sizeof(uint32_t));
>> Why do you let this pass now? Does it make sense to specify anything other than 16 or 32 bit?
>
> I use this function to build the DH_GROUP_LIST parameter, which is a
> list of 8bit items, so I remove this assertion because otherwise you
> need to do something like:

Ah yes sorry, you wrote that in the docs. Okay then.

> Thanks for your other comments. I recently have stacked many mails and
> realized that I have blocked many processes. My problem is that I have
> went back to China and started to work for a startup company. We have a
> quite tight development timetable on which I have to spend most of my
> time. I will try to fix the HIPL ECDH branch as soon as possible, but it
> is really difficult for me to give a time guarantee. If it is urgent,
> maybe someone can continue this work since the branch is already owned
> by hipl-core,

Only one of my comments was critical I think, and maybe it is no bug at
all. It's the one concerning the possible mixup of array indices. I'll
repeat the one:

On 13.09.2012 00:33, Christof Mroz wrote:
>> + for (j = 0; j < HIP_DH_GROUP_LIST_SIZE; j++) {
>> + group_id = HIP_DH_GROUP_LIST[j];
>> + for (i = 0; i < HIP_R1TABLESIZE; i++) {
>> + cookie_k = get_cookie_difficulty();
>> + hip_msg_init(&id_entry->r1_v2[j][i].buf.msg);
>> +
>> + if (hip_create_r1_v2(&id_entry->r1_v2[group_id][i].buf.msg, hit, sign,
>> + privkey, pubkey, cookie_k, group_id)) {
>> + HIP_ERROR("Unable to precreate R1_v2\n");
>> + return -1;
>> + }
>
> You use index j for hip_msg_init(), but index group_id for hip_create_r1_v2(). Is this intentional?

Good luck with the startup.