[PATCHES v2] Fix GPO unapply log

[David Mulder]

OK. I'll implement the suggestions here and post a follow up set of patches.
metze... I typically use mbox because I can just do git am mbox_file.mbox
I don't know what a test/plain file means.

On 12/07/2017 01:57 PM, Garming Sam via samba-technical wrote:
> 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(-)
>>>
>
>

-- 
David Mulder
SUSE Labs Software Engineer - Samba
dmulder at suse.com
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)