On Wed, Aug 15, 2012 at 11:02:15AM +0200, Ondrej Kos wrote:
> Ack
>
Two more nitpicks, please use strcasecmp for comparing host names and
make functions that are not exported in a header file static.
> On 08/14/2012 03:40 PM, Michal Zidek wrote:
>> On 08/14/2012 10:54 AM, Ondrej Kos wrote:
>>> Nack
>>>
>>> Working, but hard to read, use typedef for those strcmp casts.
>>>
>>> O.
>> Ok. I now use typedef for the function type to make it easier to read. I
>> also created separated functions
>> to compare user_data for each provider, so it will be easy to change it
>> without affecting other providers
>> (right now all compare functions are only wrappers around strcmp).
>>
>>> On 08/13/2012 05:25 PM, Michal Zidek wrote:
>>>> On 08/13/2012 11:13 AM, Jakub Hrozek wrote:
>>>>> On Fri, Aug 10, 2012 at 07:12:38PM +0200, Jakub Hrozek wrote:
>>>>>> On Fri, Aug 10, 2012 at 07:09:13PM +0200, Jakub Hrozek wrote:
>>>>>>> On Fri, Aug 10, 2012 at 05:13:48PM +0200, Michal Zidek
wrote:
>>>>>>>> This should fix the duplicate detection. Patch is
attached. Please
>>>>>>>> review with caution, i was in hurry.
>>>>>>>>
>>>>>>>>
https://fedorahosted.org/sssd/ticket/1472
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Michal
>>>>>>> I think the general approach with compare function is good,
but it
>>>>>>> should be a property of the fo_service. I don't think
there is any
>>>>>>> scenario where we'd like to treat two servers with same
user_data
>>>>>>> contents as the same.
>>>>>> ^^^ Sorry, this should have read "two servers from the same
service".
>>>>> You also need to amend the testsuite as well when changing the fo_
>>>>> interface, otherwise the test suite doesn't work:
>>>>>
>>>>> CC src/resolv/fail_over_tests-async_resolv.o
>>>>> src/tests/fail_over-tests.c: In function ‘test_fo_resolve_service’:
>>>>> src/tests/fail_over-tests.c:233:5: warning: passing argument 5 of
>>>>> ‘fo_add_server’ makes pointer from integer without a cast [enabled
by
>>>>> default]
>>>>> In file included from ./src/providers/dp_backend.h:26:0,
>>>>> from ./src/providers/ldap/sdap.h:25,
>>>>> from ./src/tests/common.h:31,
>>>>> from src/tests/fail_over-tests.c:35:
>>>>> ./src/providers/fail_over.h:120:5: note: expected ‘int (*)(void *,
>>>>> void *)’ but argument is of type ‘int’
>>>>> src/tests/fail_over-tests.c:233:5: error: too few arguments to
>>>>> function ‘fo_add_server’
>>>> Ok. I rewrote it, so the user_data_cmp is now part of fo_service.
>>>> If it is set to NULL, user_data is not used as part of duplicity
>>>> detection.
>>>> Also testsuite is now updated.
>>>>
>>>> New patch is attached.
>>>>
>>>> Thanks
>>>> Michal
>>>
>>
>> New patch is attached.
>>
>> Thanks
>> Michal
>>