Code review comment for lp:~rene-hummen/hipl/midauth-hipd

Thanks for your extensive review!

On 30.07.2011, at 17:01, Stefan Götz wrote:
> Review: Needs Fixing
> Hi Rene!
>
>> 4) There are no unit-tests for the hipd-related functionality as of now.
>> However, we have tested the code extensively in three demos and I will add the
>> tests at a later point in time.
>
> Sorry, but I'm skeptical :-) As per our current policy, I cannot approve this without unit tests.

Well, you are right. I'll add them now.

>> === modified file 'firewall/midauth.h'
>> === modified file 'lib/core/solve.c'
>> === modified file 'lib/core/solve.h'
>
> Please don't forget to update the Copyright dates when you modify files with
> Copyright headers. Scatterbrains like myself may want to try out the pre-commit
> hook in https://code.launchpad.net/~stefan.goetz/hipl/commitguard that
> automatically checks for that.

I just started using your commit guard after finishing this branch. Future commit should be bullet proof.

>> +static int handle_challenge_request_param(UNUSED const uint8_t packet_type,
>> + UNUSED const uint32_t ha_state,
>> + struct hip_packet_context *ctx)
>
> [M] policy: please ensure full const correctness for the 'ctx' function argument

Handle functions currently require the non-constness. This should be fixed in trunk.

>> +int hip_midauth_puzzle_seed(const uint8_t opaque[],
>> + const uint8_t opaque_len,
>> + uint8_t puzzle_value[PUZZLE_LENGTH])
>
> [M] lack of const correctness

I don't know how to make this more const correct. The prototype uses the pointer notation with complete const-correctness instead of arrays though.

René

--
Dipl.-Inform. Rene Hummen, Ph.D. Student
Chair of Communication and Distributed Systems
RWTH Aachen University, Germany
tel: +49 241 80 20772
web: http://www.comsys.rwth-aachen.de/team/rene-hummen/