[sssd PR#70][opened] check_duplicate: check name member before using it

[fidencio]

URL: https://github.com/SSSD/sssd/pull/70
Author: sumit-bose
 Title: #70: check_duplicate: check name member before using it
Action: opened

PR body:
"""
The second patch resolves https://fedorahosted.org/sssd/ticket/3231 and adds a
test for the issue.

The first patches fixes a potential memory leak which so far was only relevant
in the tests.
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/70/head:pr70
git checkout pr70

[sssd PR#70][comment] check_duplicate: check name member before using it

[fidencio]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

celestian commented:
"""
The patch set LGTM.
I am waiting for CI and I would like to test it manually.
"""

See the full comment at https://github.com/SSSD/sssd/pull/70#issuecomment-258778350

[sssd PR#70][synchronized] check_duplicate: check name member before using it

[fidencio]

URL: https://github.com/SSSD/sssd/pull/70
Author: sumit-bose
 Title: #70: check_duplicate: check name member before using it
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/70/head:pr70
git checkout pr70

[sssd PR#70][comment] check_duplicate: check name member before using it

[fidencio]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

celestian commented:
"""
I tested manually with IPA provider. It works and I was informed about attribute colision:
```
[sdap_extend_map] (0x0010): Attribute entryUSN (abc in LDAP) is already used by SSSD, please choose a different cache name
```
CI tests passed:
http://sssd-ci.duckdns.org/logs/job/56/33/summary.html

=> ACK
"""

See the full comment at https://github.com/SSSD/sssd/pull/70#issuecomment-259085686

[sssd PR#70][+Accepted] check_duplicate: check name member before using it

[fidencio]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

Label: +Accepted

[sssd PR#70][-Accepted] check_duplicate: check name member before using it

[fidencio]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

Label: -Accepted

[sssd PR#70][+Changes requested] check_duplicate: check name member before using it

[fidencio]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

Label: +Changes requested

[sssd PR#70][comment] check_duplicate: check name member before using it

[fidencio]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

lslebodn commented:
"""
I would appreciate integration test for the crash.
Feel free to extend `test_extra_attribute_already_exists` or create new one.
"""

See the full comment at https://github.com/SSSD/sssd/pull/70#issuecomment-259087435

[sssd PR#70][comment] check_duplicate: check name member before using it

[fidencio]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

sumit-bose commented:
"""
@lslebodn, the patch adds a new unit-test test_extra_opts_empty_name() which covers the crash. What would be the benefit of checking it in the integration tests as well?
"""

See the full comment at https://github.com/SSSD/sssd/pull/70#issuecomment-259091650

[sssd PR#70][comment] check_duplicate: check name member before using it

[fidencio]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

celestian commented:
"""
@lslebodn, Lukas, are you satisfied by Sumit's explanation?
"""

See the full comment at https://github.com/SSSD/sssd/pull/70#issuecomment-261941593

[sssd PR#70][comment] check_duplicate: check name member before using it

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

jhrozek commented:
"""
I'm not sure if it's even possible to write an integration test because even after the patch, sssd doesn't start, it 'just' doesn't segfault.
"""

See the full comment at https://github.com/SSSD/sssd/pull/70#issuecomment-278992834

[sssd PR#70][+Accepted] check_duplicate: check name member before using it

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

Label: +Accepted

[sssd PR#70][-Changes requested] check_duplicate: check name member before using it

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

Label: -Changes requested

[sssd PR#70][comment] check_duplicate: check name member before using it

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

lslebodn commented:
"""
On (10/02/17 08:35), Jakub Hrozek wrote:
> I'm not sure if it's even possible to write an integration test because even after the patch, sssd doesn't start, it 'just' doesn't segfault.

Everything is possible.

LS

"""

See the full comment at https://github.com/SSSD/sssd/pull/70#issuecomment-278998867

[sssd PR#70][comment] check_duplicate: check name member before using it

[fidencio]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

fidencio commented:
"""
Can we have this one pushed by @sumit-bose and @jhrozek review?
"""

See the full comment at https://github.com/SSSD/sssd/pull/70#issuecomment-281199204

[sssd PR#70][comment] check_duplicate: check name member before using it

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

lslebodn commented:
"""
On (20/02/17 14:43), fidencio wrote:
> Can we have this one pushed by @sumit-bose and @jhrozek review?

I would still prefer to see an integration test.

Feel free to write one :-)

LS

"""

See the full comment at https://github.com/SSSD/sssd/pull/70#issuecomment-281283905

[sssd PR#70][comment] check_duplicate: check name member before using it

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

jhrozek commented:
"""
On Tue, Feb 21, 2017 at 01:02:45AM -0800, lslebodn wrote:
> On (20/02/17 14:43), fidencio wrote:
> > Can we have this one pushed by @sumit-bose and @jhrozek review?
>
> I would still prefer to see an integration test.
>
> Feel free to write one :-)

I tried to, really, but I haven't been able to figure out how to
distinguish nicely between a crash and a failure to start. And after
about an hour I gave up because there already is a unit test so we won't
regress and honestly, I have more pressing work.

So unless someone else submits an integration test, I will push this
patch before the next upstream release so that we no longer have this
bug.

"""

See the full comment at https://github.com/SSSD/sssd/pull/70#issuecomment-281291185

[sssd PR#70][comment] check_duplicate: check name member before using it

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

jhrozek commented:
"""
Actually, let's push this PR now, there is a test so we won't regress
"""

See the full comment at https://github.com/SSSD/sssd/pull/70#issuecomment-281643957

[sssd PR#70][comment] check_duplicate: check name member before using it

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

jhrozek commented:
"""

master:
        454cf0c3808a9f6a0c9f79e9796e17c58907ee6c
        08bf6b4a281ef4308119dccbba4e86cf28b505d2 
    sssd-1-14:
        c14980e81253aaec2fddb4f794fb1eb39167e885
        bb4b624bfb3a08fc3b2989d0cce05afd2c3d4843

"""

See the full comment at https://github.com/SSSD/sssd/pull/70#issuecomment-281645844

[sssd PR#70][+Pushed] check_duplicate: check name member before using it

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/70
Title: #70: check_duplicate: check name member before using it

Label: +Pushed

[sssd PR#70][closed] check_duplicate: check name member before using it

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/70
Author: sumit-bose
 Title: #70: check_duplicate: check name member before using it
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/70/head:pr70
git checkout pr70