[PATCHES v2] Fix GPO unapply log

Garming Sam garming at catalyst.net.nz
Thu Dec 7 20:57:40 UTC 2017


Hi,

I'm fine with the changes in gpclass.py. I've also checked that all the
tests pass.

In find_attr_val, you declared a variable in the for loop, which we
don't allow right now. But as metze has said, you should just replace
this function with the existing helper methods in ldb.


Cheers,

Garming

On 07/12/17 19:57, Stefan Metzmacher via samba-technical wrote:
> Hi David,
>
> would it be possible that you attach patches
> as test/plain instead of application/mbox?
> That would make it much easier to review the patches (at least for me)
>
> Can you please make use of existing helper functions in
> source4/torture/gpo/apply.c instead of introducing find_attr_val()
> I guess ldb_msg_find_attr_as_string() can be used instead.
>
> Using val = ldb_msg_find_ldb_val() and then
> (char*)val->data should be also be replaced by
> ldb_msg_find_attr_as_string().
>
> ldb_msg_find_attr_as_[u]int*() together with torture_assert_int_equal()
> should be used instead of using atoi() and torture_assert().
>
> Please let someone else review the gpclass.py I don't have time to
> wrap my head around this currently.
>
> Thanks!
> metze
>
> Am 06.12.2017 um 19:23 schrieb David Mulder via samba-technical:
>> Fixes for 2 bugs in the gpo unapply.
>> * The apply log was being left empty. Apparently pointers to tree
>> elements can't be stored in an object, they somehow are disconnected
>> from the main tree and nothing gets logged. These have to be found each
>> time we want to add something. This is probably a bug in python ElementTree.
>> * We should only commit the earliest change from a gpo to the log.
>> Otherwise we overwrite the original value written by that gpo, and then
>> we tattoo the setting on unapply.
>> * Adds testing so this isn't missed again.
>>
>>  python/samba/gpclass.py               |  67 ++++++++++++++++++++----------------
>>  source4/scripting/bin/samba_gpoupdate |   4 +--
>>  source4/torture/gpo/apply.c           | 108 +++++++++++++++++++++++++++++++++++++++++++++++++----------
>>  3 files changed, 130 insertions(+), 49 deletions(-)
>>
>




More information about the samba-technical mailing list