[Freeipa-devel] [bind-dyndb-ldap PR#11][opened] Coverity: fix REVERSE_INULL for pevent->inst

[tomaskrizek]

URL: https://github.com/freeipa/bind-dyndb-ldap/pull/11
Author: tomaskrizek
Title: #11: Coverity: fix REVERSE_INULL for pevent->inst
Action: opened

PR body:
"""
With the DynDB API changes, the ldap instance is acquired
differently. Previously, obtaining the instance could fail when
LDAP was disconnecting, thus the NULL check was necessary in the
cleanup part.

Now, inst is obtained directly from the API. I'm not sure what is
the exact behaviour in edge cases such as LDAP disconnecting, so
I perform the NULL check a bit earlier, just to be safe.
"""

To pull the PR as Git branch:
git remote add ghbind-dyndb-ldap https://github.com/freeipa/bind-dyndb-ldap
git fetch ghbind-dyndb-ldap pull/11/head:pr11
git checkout pr11

[tomaskrizek]

URL: https://github.com/freeipa/bind-dyndb-ldap/pull/11
Title: #11: Coverity: fix REVERSE_INULL for pevent->inst

tomaskrizek commented:
"""
@pemensik Hi, could you take a quick look at this change?

I ran coverity and the issues were fixed.

It might also be possible to remove the REQUIRE, but since I'm not sure whether `inst` is always non null in the new dyndb workflow, I added the check just to be sure.
"""

See the full comment at https://github.com/freeipa/bind-dyndb-ldap/pull/11#issuecomment-290026409

[pemensik]

URL: https://github.com/freeipa/bind-dyndb-ldap/pull/11
Title: #11: Coverity: fix REVERSE_INULL for pevent->inst

pemensik commented:
"""
Hi Tomáš,

I did not find any place which could result with inst == NULL. ldap_sync_prepare contains proper REQUIRE(inst != null) and all other function uses its result stored in ls_private. I think null check does not harm anything. It is used in BIND to catch unexpected problems after code changes. I would suggest to place one require at the beginning of syncrepl_update function also.

Overall I think it should be merged.
"""

See the full comment at https://github.com/freeipa/bind-dyndb-ldap/pull/11#issuecomment-290398226

[tomaskrizek]

URL: https://github.com/freeipa/bind-dyndb-ldap/pull/11
Title: #11: Coverity: fix REVERSE_INULL for pevent->inst

tomaskrizek commented:
"""
Thanks for the review, Petr!

I added the check to `syncrepl_update` as well.
"""

See the full comment at https://github.com/freeipa/bind-dyndb-ldap/pull/11#issuecomment-291178049

[tomaskrizek]

URL: https://github.com/freeipa/bind-dyndb-ldap/pull/11
Author: tomaskrizek
Title: #11: Coverity: fix REVERSE_INULL for pevent->inst
Action: synchronized

To pull the PR as Git branch:
git remote add ghbind-dyndb-ldap https://github.com/freeipa/bind-dyndb-ldap
git fetch ghbind-dyndb-ldap pull/11/head:pr11
git checkout pr11

[tomaskrizek]

URL: https://github.com/freeipa/bind-dyndb-ldap/pull/11
Title: #11: Coverity: fix REVERSE_INULL for pevent->inst

Label: +ack

[tomaskrizek]

URL: https://github.com/freeipa/bind-dyndb-ldap/pull/11
Title: #11: Coverity: fix REVERSE_INULL for pevent->inst

tomaskrizek commented:
"""
master:
- 13b185182aeb48562cf63251b84bcf910b57a0fc
"""

See the full comment at https://github.com/freeipa/bind-dyndb-ldap/pull/11#issuecomment-292221521

[tomaskrizek]

URL: https://github.com/freeipa/bind-dyndb-ldap/pull/11
Author: tomaskrizek
Title: #11: Coverity: fix REVERSE_INULL for pevent->inst
Action: closed

To pull the PR as Git branch:
git remote add ghbind-dyndb-ldap https://github.com/freeipa/bind-dyndb-ldap
git fetch ghbind-dyndb-ldap pull/11/head:pr11
git checkout pr11

[tomaskrizek]

URL: https://github.com/freeipa/bind-dyndb-ldap/pull/11
Title: #11: Coverity: fix REVERSE_INULL for pevent->inst

Label: +pushed