[Freeipa-devel] [PATCH 0239-0243] Refactor ldap_parse_master_zoneentry()

[Petr Spacek]

On 3.6.2014 10:41, Petr Spacek wrote:
> On 6.5.2014 22:11, Lukas Slebodnik wrote:
>> On (06/05/14 17:15), Petr Spacek wrote:
>>> On 6.5.2014 14:41, Tomas Hozza wrote:
>>>> ----- Original Message -----
>>>>> Hello,
>>>>>
>>>>> This patch set attempts to move ldap_parse_master_zoneentry() a little bit
>>>>> closer to sane code.
>>>>>
>>>>> It is preparation for
>>>>> https://fedorahosted.org/bind-dyndb-ldap/ticket/56
>>>>>
>>>>> --
>>>>> Petr^2 Spacek
>>>>>
>>>>
>>>> Patches look good.
>>>>
>>>> ACK.
>>>>
>>>> ACKing of version 2 of the patch 242 will follow. The patch 243 introduced
>>>> new compilation
>>>> warning that Peter is aware of. Unfortunately we are unable to find the
>>>> root cause of it,
>>>> so leaving it as is for now...
>>>
>>> I managed to find & fix one problem (see new version of the patch
>>> 243) but GCC still complains.
>>>
>>> ../../src/ldap_helper.c: In function 'update_zone':
>>> ../../src/ldap_helper.c:2334:34: error: 'data_changed' may be used
>>> uninitialized in this function [-Werror=maybe-uninitialized]
>>>   if (sync_state == sync_finished && data_changed == ISC_TRUE)
>>>                                   ^
>>> ../../src/ldap_helper.c:2218:16: note: 'data_changed' was declared here
>>>   isc_boolean_t data_changed;
>>>
>>> On my machine with gcc-4.8.2-7.fc20.x86_64 this happens only with -O2.
>>>
>> The same problem with -01,-Os,-O2 or -O3
>>
>> I doubt it is false possibive, because I could reproduce it even with
>> gcc-4.9.0-1.fc21.x86_64
>>
>>> I'm not able to reproduce this with clang-3.4-6.fc20.x86_64 but it is
>>> no so surprising - Clang didn't catch even the first case (fixed by
>>> patch version 2).
>>>
>>> Any hint what is wrong or how to refactor code will be appreciated! ;-)
>>>
>> I think it can be some kind of optimization in function zone_sync_apex.
>> You can try to debug this function with plugin "-O2"-build :-)
>>
>> The warning can be suppresed with initialising variable before the 1st CHECK.
>> It will not work if you try to initialize later.
>
> Yesterday I have discussed this with jkratoch. We weren't able to find out
> case where would initialization in ldap_parse_master_zoneentry() cause any
> problem so I have added initialization there.

This is delayed push notice:
1aff693f77ef3f2e7f059b52becb5b178eb7b194
04fa577e67543a0b07db329e1ad7fb86c48896ff
2e45aa1d7b83bc33e31b87e919651530944553fb
4fcbaaabf94d9bf2f6942f2ebbc40fed9d2c41a6
f06a0a7375e97d7d275290d8331172fea73be6a4

-- 
Petr^2 Spacek