[PATCH 7/7] python: wrap and use notmuch_database_destroy as destructor

Justus Winter 4winter at informatik.uni-hamburg.de
Sun Apr 22 05:06:20 PDT 2012


Quoting Austin Clements (2012-04-12 19:02:49)
> Quoth Justus Winter on Mar 21 at  1:55 am:
> > Adapt the go bindings to the notmuch_database_close split.
> 
> Typo.

Duh >,<

> > Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
> > ---
> >  bindings/python/notmuch/database.py |   17 +++++++++++++++--
> >  1 files changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/bindings/python/notmuch/database.py b/bindings/python/notmuch/database.py
> > index 44d40fd..9a1896b 100644
> > --- a/bindings/python/notmuch/database.py
> > +++ b/bindings/python/notmuch/database.py
> > @@ -218,9 +218,22 @@ class Database(object):
> >      _close.restype = None
> >  
> >      def close(self):
> > -        """Close and free the notmuch database if needed"""
> > -        if self._db is not None:
> > +        '''
> > +        Closes the notmuch database.
> > +        '''
> > +        if self._db:
> >              self._close(self._db)
> > +
> > +    _destroy = nmlib.notmuch_database_destroy
> > +    _destroy.argtypes = [NotmuchDatabaseP]
> > +    _destroy.restype = None
> > +
> > +    def destroy(self):
> 
> Should this be __del__?  The existing __del__ is certainly wrong with
> this change, since it only closes the database and doesn't free it.  I
> think this function is exactly what you want __del__ to be now.
> 
> (I think it also doesn't make sense to expose notmuch_database_destroy
> as a general, public method since it will free all of the other C
> objects out from under the bindings, resulting in exactly the double
> free-type crashes that you're trying to avoid.  It appears that none
> of the other Python classes have a destroy method.)

Yes, of course...

I'm going to send an rebased and updated patch series as a follow up
to this message. Thanks for the input everyone :)

Justus


More information about the notmuch mailing list