[sssd PR#1009][opened] Use ndr_pull_steal_switch_value for modern samba versions

[noelpower]

   URL: https://github.com/SSSD/sssd/pull/1009
Author: noelpower
 Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions
Action: opened

PR body:
"""
commit bc56b10aea999284458dcc293b54cf65288e325d attempted to
fix the build error resulting from removal of 'ndr_pull_get_switch'

However this just basically leaves the old code in place, the
reason for the change in the samba code was to address a possible
NDR crash from possible illegal large array allocations, please
see https://bugzilla.samba.org/show_bug.cgi?id=13876

This change uses the new replacement method
'ndr_pull_steal_switch_value' if available and otherwise puts
back the old method (which better describes what the code is doing)

Signed-off-by: Noel Power <noel.power(a)suse.com&gt;
"""

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

[sssd PR#1009][comment] Use ndr_pull_steal_switch_value for modern samba versions

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions

pbrezina commented:
"""
Thank you. The code is copied from samba sources so perhaps it would make sense to update
the whole sources instead of just this function? From a quick look it seems that there
were more changes over the time.
"""

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

[sssd PR#1009][comment] Use ndr_pull_steal_switch_value for modern samba versions

[noelpower]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions

noelpower commented:
"""
>  Thank you. The code is copied from samba sources so perhaps it would
> make sense to update the whole sources instead of just this function? From a quick look it
> seems that there were more changes over the time.
>  
I can't speak for the rest of the sources, I am not familiar with the sssd code. In
this one example replacing ndr_pull_get_switch_value with ndr_token_peek post samba 4.12
just isn't wise given the bug that is being addressed.

I'm guessing from the comment above that this code was copied from the samba source
tree at some time. In samba however this type of code is generated from idl and so is
somewhat dynamic, however the generated code is always in step with the version of samba
it is used with (since it is in-tree).

While it is possible to re-copy (or refresh) the sssd source from later versions of
generated code from the samba source tree your mileage may vary when trying to build with
different versions of samba.

The correct solution would be for sssd to generate the necessary code itself as part of
the build, presumably there are technical  or practical reasons why this hasn't been
done .
"""

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

[sssd PR#1009][comment] Use ndr_pull_steal_switch_value for modern samba versions

[noelpower]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions

noelpower commented:
"""
actually I wonder are the version specific elements  necessary, need to check how long
ndr_pull_steal_switch_value is around

"""

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

[sssd PR#1009][synchronized] Use ndr_pull_steal_switch_value for modern samba versions

[noelpower]

   URL: https://github.com/SSSD/sssd/pull/1009
Author: noelpower
 Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions
Action: synchronized

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

[sssd PR#1009][comment] Use ndr_pull_steal_switch_value for modern samba versions

[noelpower]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions

noelpower commented:
"""
first commit refreshes the affected methods from samba 4.12, still need the version check
as abi for ndr_pull_steal_switch_value has changed >= 4.12

second commit attempts to refresh the remainder of the methods in
src/providers/ad/ad_gpo_ndr.c
Not sure of the value of the second commit, not much changed here (also should be
carefully inspected as the formatting etc. is different for sssd code and I manually
changed what I could visually see as different)
These changes are not tested (except for compile)
"""

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

[sssd PR#1009][synchronized] Use ndr_pull_steal_switch_value for modern samba versions

[noelpower]

   URL: https://github.com/SSSD/sssd/pull/1009
Author: noelpower
 Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions
Action: synchronized

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

[sssd PR#1009][synchronized] Use ndr_pull_steal_switch_value for modern samba versions

[noelpower]

   URL: https://github.com/SSSD/sssd/pull/1009
Author: noelpower
 Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions
Action: synchronized

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

[sssd PR#1009][edited] WIP - Use ndr_pull_steal_switch_value for modern samba versions

[noelpower]

   URL: https://github.com/SSSD/sssd/pull/1009
Author: noelpower
 Title: #1009: WIP - Use ndr_pull_steal_switch_value for modern samba versions
Action: edited

 Changed field: title
Original value:
"""
Use ndr_pull_steal_switch_value for modern samba versions
"""

[sssd PR#1009][comment] WIP - Use ndr_pull_steal_switch_value for modern samba versions

[noelpower]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: WIP - Use ndr_pull_steal_switch_value for modern samba versions

noelpower commented:
"""
I'm not sure I trust the successful checks above, previously
'test_mod_group_gid' failed a couple of times (on fedora32) however it passes
locally for me on my own machine
"""

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

[sssd PR#1009][edited] [WIP] - Use ndr_pull_steal_switch_value for modern samba versions

[noelpower]

   URL: https://github.com/SSSD/sssd/pull/1009
Author: noelpower
 Title: #1009: [WIP] - Use ndr_pull_steal_switch_value for modern samba versions
Action: edited

 Changed field: title
Original value:
"""
WIP - Use ndr_pull_steal_switch_value for modern samba versions
"""

[sssd PR#1009][synchronized] [WIP] - Use ndr_pull_steal_switch_value for modern samba versions

[noelpower]

   URL: https://github.com/SSSD/sssd/pull/1009
Author: noelpower
 Title: #1009: [WIP] - Use ndr_pull_steal_switch_value for modern samba versions
Action: synchronized

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

[sssd PR#1009][comment] [WIP] - Use ndr_pull_steal_switch_value for modern samba versions

[noelpower]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: [WIP] - Use ndr_pull_steal_switch_value for modern samba versions

noelpower commented:
"""
so, it seems the earlier failures (with ingtest 'test_mod_group_gid') must have
been unrelated to the changes in this pull request. In a local build I pumped in debug
messages in any of the methods that were modified and forced that individual test to fail.
I could find no sign of the debug messages (which was at level 0) in the logs.
Additionally even inserted an 'abort()' statement into the same functions and ran
a full 'make intgcheck.
Results with and without patches were the same, nothing extra failed and no sign of of the
debug level 0 messages I inserted. (interestingly for some reason in my env I have a
single test that fails with/without these patches) but that is another issue I guess
"""

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

[sssd PR#1009][edited] Use ndr_pull_steal_switch_value for modern samba versions

[noelpower]

   URL: https://github.com/SSSD/sssd/pull/1009
Author: noelpower
 Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions
Action: edited

 Changed field: title
Original value:
"""
[WIP] - Use ndr_pull_steal_switch_value for modern samba versions
"""

[sssd PR#1009][comment] Use ndr_pull_steal_switch_value for modern samba versions

[noelpower]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions

noelpower commented:
"""
so, it seems the earlier failures (with ingtest 'test_mod_group_gid') must have
been unrelated to the changes in this pull request. In a local build I pumped in debug
messages in any of the methods that were modified and forced that individual test to fail.
I could find no sign of the debug messages (which was at level 0) in the logs.
Additionally even inserted an 'abort()' statement into the same functions and ran
a full 'make intgcheck.
Results with and without patches were the same, nothing extra failed and no sign of of the
debug level 0 messages I inserted. (interestingly for some reason in my env I have a
single test that fails with/without these patches) but that is another issue I guess

when this was failing there were 2 failing jobs, I think the one with the failed
'intgcheck' job was fedora32 (but I am not sure now) and additionally sssd-ci I
don't know what was failing in sssd-ci (or if it is some sort of summary of other
jobs) 
"""

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

[sssd PR#1009][comment] Use ndr_pull_steal_switch_value for modern samba versions

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions

pbrezina commented:
"""
Thank you. The patches looks good to me, I will just run some more tests.
"""

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

[sssd PR#1009][comment] Use ndr_pull_steal_switch_value for modern samba versions

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions

pbrezina commented:
"""
Manual testing went fine, I'm also running downstream automated tests.
"""

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

[sssd PR#1009][comment] Use ndr_pull_steal_switch_value for modern samba versions

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions

pbrezina commented:
"""
Ack. Thank you.
"""

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

[sssd PR#1009][+Ready to push] Use ndr_pull_steal_switch_value for modern samba versions

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions

Label: +Ready to push

[sssd PR#1009][+Accepted] Use ndr_pull_steal_switch_value for modern samba versions

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions

Label: +Accepted

[sssd PR#1009][comment] Use ndr_pull_steal_switch_value for modern samba versions

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions

pbrezina commented:
"""
* `master`
    * c031adde4f532f39845a0efd78693600f1f8b2f4 - ad_gpo_ndr.c: refresh ndr_ methods from
samba-4.12
    * 1fdd8fa2fded1985fbfc6aa67394eebcdbb6a2fc - Use ndr_pull_steal_switch_value for
modern samba versions

"""

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

[sssd PR#1009][+Pushed] Use ndr_pull_steal_switch_value for modern samba versions

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions

Label: +Pushed

[sssd PR#1009][-Accepted] Use ndr_pull_steal_switch_value for modern samba versions

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions

Label: -Accepted

[sssd PR#1009][-Ready to push] Use ndr_pull_steal_switch_value for modern samba versions

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/1009
Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions

Label: -Ready to push

[sssd PR#1009][closed] Use ndr_pull_steal_switch_value for modern samba versions

[pbrezina]

   URL: https://github.com/SSSD/sssd/pull/1009
Author: noelpower
 Title: #1009: Use ndr_pull_steal_switch_value for modern samba versions
Action: closed

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