[PATCHES v2] Fix GPO unapply log

[jim]

He meant text/plain file attachment format.

On 12/8/2017 9:16 AM, David Mulder via samba-technical wrote:
> 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(-)
>>>>
>>