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

Revision history for this message
Christof Mroz (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.

> Changing this is outside the scope of this merge request and it is not
> at all clear that a GNU-specific linker flag is the better solution.

I'd really appreciate a hint... the best I could come up with was the
LD_PRELOAD solution (see referenced thread), which AFAIK was invented
for cases like these: The need to intercept library calls.

You'd have to build a separate shared object to be linked with the tests
at runtime though, which is no rocket science but would require changes
and debugging in the build system nevertheless.

If anybody is willing to implement it anyway, I'd happily elaborate on
that idea as it would allow us to constrain the mock functions to those
tests actually using them.

« Back to merge proposal