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

[Lukas Slebodnik]

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.

--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2116,6 +2116,7 @@ zone_sync_apex(const ldap_instance_t * const inst,
        isc_uint32_t curr_serial;

        INIT_LIST(rdatalist);
+       *data_changed = ISC_FALSE;
        CHECK(setting_get_str("fake_mname", inst->local_settings,
                              &fake_mname));
        CHECK(ldap_parse_rrentry(inst->mctx, entry, &name, fake_mname,

>--
>Petr^2 Spacek

LS