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

Forgot to CC launchpad.

-------- Original Message --------
Subject: [hipl-dev] Re: [Merge]
lp:~midauth-pisa-devs/hipl/midauth-firewall into lp:hipl
Date: Wed, 26 Oct 2011 23:16:18 +0200
From: Christof Mroz <email address hidden>
Reply-To: <email address hidden>
To: <email address hidden>

On 26.10.2011 22:57, Diego Biurrun wrote:
> On Tue, Oct 25, 2011 at 11:43:43PM +0200, Christof Mroz wrote:
>> On 25.10.2011 23:02, Diego Biurrun wrote:
>>> On Tue, Oct 25, 2011 at 02:35:31PM +0000, René Hummen wrote:
>>>> On 25.10.2011, at 15:24, Diego Biurrun wrote:
>>>>> On Tue, Oct 25, 2011 at 02:28:06PM +0200, René Hummen wrote:
>>>>>> On 20.10.2011, at 14:15, Diego Biurrun wrote:
>>>>>>> On Wed, Oct 19, 2011 at 01:17:30PM +0000, René Hummen wrote:
>>>>>>>
>>>>>>>> --- Makefile.am 2011-10-17 18:14:10 +0000
>>>>>>>> +++ Makefile.am 2011-10-19 13:13:48 +0000
>>>>>>>> @@ -253,7 +256,7 @@
>>>>>>>>
>>>>>>>> -test_check_firewall_LDFLAGS = -ldl
>>>>>>>> +test_check_firewall_LDFLAGS = -ldl -z muldefs
>>>>>>>
>>>>>>> We have solved this differently elsewhere by #including the .c file to
>>>>>>> unit-test static functions.
>>>>>>
>>>>>> This has been discussed previously on the list:
>>>>>> http://www.freelists.org/post/hipl-dev/Mock-functions-for-hipl-unit-tests
>>>>>
>>>>> But we currently don't favor that solution. If that needs changing,
>>>>> it's a topic for a separate merge request and would need to be changed
>>>>> in all places.
>>>>
>>>> The change was necessary in order to comply with our unit-test for new
>>>> code policy as unit-tests would otherwise not link correctly. The only
>>>> other option I see right now is to merge the code without unit-tests.
>>>
>>> No, the change is not necessary. As I said before, we have solved
>>> this differently elsewhere by #including the .c file to unit-test
>>> static functions.
>>
>> Including the .c file allows you to access static identifiers in
>> unit tests, but the libipq problem (see referenced thread) is a
>> different one.
>> Apparently, Hendrik already tried to mimic the other (working) tests
>> already without success.
>
> I'd like to know what is failing exactly and how before making
> a judgement here.

See the referenced thread for details, but essentially we need to mock
ipq_get_packet() which is usually provided by linking -lipq.
Strangely, while our usual approach (simply exporting a function with
exact same signature in an object file) works for libc, it does fail
with a "multiple definition" error for explicitly linked-in libs like
libipq.
Making libipq just as "magic" as libc would solve the problem, if that's
possible.