Code review comment for lp:~midauth-pisa-devs/hipl/midauth-firewall

Revision history for this message
René Hummen (rene-hummen) wrote :

Begin forwarded message:
> From: René Hummen <email address hidden>
> Date: 2. November 2011 18:44:49 MEZ
> To: <email address hidden>
> Subject: Re: [hipl-dev] Re: [Merge] lp:~midauth-pisa-devs/hipl/midauth-firewall into lp:hipl
>
> On 27.10.2011, at 00:50, Christof Mroz wrote:
>> On 27.10.2011 00:20, Diego Biurrun wrote:
>>>> See the referenced thread for details, but essentially we need to
>>>> mock ipq_get_packet() which is usually provided by linking -lipq.
>>>
>>> What I don't get is the reason why we need to mock that function
>>> in the first place. Probably I am missing something totally
>>> obvious.
>>
>> Looking at the patch and judging from my memory, I think it's called by hipfw_init_context().
>> On the one hand, much of the firewall code makes heavy direct use of ipq structures (they're even part of the context structure itself), but now that you mention it... maybe hipfw_init_context() could still be split into two functions such that you can just hand it a fake ipq_packet pointer.
>> What do you think, René?
>
> I already thought about splitting up the code when implementing the unit test. But then I realized that mocking of the ipq functionality actually worked and decided to keep hip_fw_init_context() as it is. It's no problem to split up functionality though. Just one thing: Does anybody have a simple way to convert HIP_HEXDUMP output to char values like "\x00\x30\xF9\x16\x00\x88\xFF\xFF\x00". Last time I did that manually and realized that this was a bad idea. It would be perfect if we could specify the line length of the converted output in order to prevent long lines. If we have such a script, it's definitely worth adding it to our repository.
>
>> This would just postpone a more general problem of course, but maybe I can look into it more thoroughly when the lectures are over.
>
> Thanks, that would be great.

« Back to merge proposal