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

[Petr Spacek]

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.

-- 
Petr^2 Spacek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bind-dyndb-ldap-pspacek-0243-3-Refactor-zone-apex-synchronization-and-serial-mainte.patch
Type: text/x-patch
Size: 12021 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140603/17ff7e16/attachment.bin>