[PATCH] pytalloc and pidl changes for refcounting and incorrect msDS-IntID handling

[Andrew Bartlett]

On Tue, 2016-02-16 at 14:28 +0100, Stefan Metzmacher wrote:
> Am 16.02.2016 um 04:25 schrieb Andrew Bartlett:
> > On Wed, 2016-01-06 at 13:10 +1300, Andrew Bartlett wrote:
> > >  
> > > Thanks.  I also have had a failure that indicates our python
> > > reference
> > > counting issue may be biting us again.  I'm looking into that.
> > > 
> > > Andrew Bartlett
> > 
> > The attached patches appear to address the segfaults by sorting out
> > our
> > reference handling - we need to track both the talloc context of
> > the
> > Samba object and the reference handle used by python.
> > 
> > I even have tests to show we are handling the references correctly!
> 
> Thanks!
> 
> There's one unused tmp_ctx variable you just create and free...
> 
> > This all seems fairly sane, until you realise that pytalloc.h is a
> > PUBLIC header.  I've tried some things to fix that, but none of it
> > is
> > easy, or even practical, so I'm posting this to the list to seek
> > advice.  
> > 
> > Is it reasonable to make an ABI break on pytalloc.h without
> > changing
> > the talloc major version?
> > 
> 
> I think the goal should be to remove *every* PyObject_HEAD from any
> header
> file! That means all internal object state is private and instead of
> ugly
> macros like pytalloc_get_mem_ctx(), pytalloc_get_ptr(),
> PyCredentials_AsCliCredentials()
> and many others need to be real functions.

I've done this.

> For objects derived from "talloc.Object", we should provide a
> pytalloc_Ready() as wrapper to PyType_Ready(), that fills in the
> required
> magic to the PyTypeObject.

I chose to just expose a size function, but yes, this is essentially
what we now have.

> In order to prevent ABI problems we can think of adding new functions
> and keep (and deprecate) the broken once. Maybe we can provide the
> abstract
> class as something like "talloc.BaseObject".

I've added new functions, and used the talloc.BaseObject name.  I've
removed the macros, which creates an API (but not ABI) break if callers
used pytalloc_Object as parts of Samba did and build strictly, because
the types will mismatch.

I think this is the best way forward, given the limited audience and
that it avoids leaving the macros around forever. 

I've added tests, both for the PIDL changes and in test_pytalloc.c

The pytalloc changes are attached again, for your consideration.

Andrew Bartlett

-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba





-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-pydsdb-Do-not-call-py_return_ndr_struct-on-ldb.Messa.patch
Type: text/x-patch
Size: 2917 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160222/b6bd6b13/0001-pydsdb-Do-not-call-py_return_ndr_struct-on-ldb.Messa.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-python-Correct-python-bindings-not-to-use-pytalloc_O.patch
Type: text/x-patch
Size: 56513 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160222/b6bd6b13/0002-python-Correct-python-bindings-not-to-use-pytalloc_O.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-pytalloc-Add-new-BaseObject.patch
Type: text/x-patch
Size: 22246 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160222/b6bd6b13/0003-pytalloc-Add-new-BaseObject.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-talloc-Change-pytalloc_get_ptr-pytalloc_get_mem_ctx-.patch
Type: text/x-patch
Size: 3663 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160222/b6bd6b13/0004-talloc-Change-pytalloc_get_ptr-pytalloc_get_mem_ctx-.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-pytalloc-Add-pytalloc_get_ptr_mem_ctx-for-talloc.Bas.patch
Type: text/x-patch
Size: 1479 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160222/b6bd6b13/0005-pytalloc-Add-pytalloc_get_ptr_mem_ctx-for-talloc.Bas.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-talloc-Bump-version-number.patch
Type: text/x-patch
Size: 1854 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160222/b6bd6b13/0006-talloc-Bump-version-number.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160222/b6bd6b13/signature.sig>