Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
To: Austin Clements <amdragon@MIT.EDU>, 
From: Justus Winter <4winter@informatik.uni-hamburg.de>
In-Reply-To: <20120412170249.GG13549@mit.edu>
References:
 <1332291311-28954-1-git-send-email-4winter@informatik.uni-hamburg.de>
	<1332291311-28954-8-git-send-email-4winter@informatik.uni-hamburg.de>
	<20120412170249.GG13549@mit.edu>
Message-ID: <20120422120620.26225.40778@thinkbox.jade-hamburg.de>
User-Agent: alot/0.3+
Subject: Re: [PATCH 7/7] python: wrap and use notmuch_database_destroy as
	destructor
Date: Sun, 22 Apr 2012 14:06:20 +0200
Cc: notmuch@notmuchmail.org
Precedence: list

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@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/notm=
uch/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 =3D 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 =3D nmlib.notmuch_database_destroy
> > +    _destroy.argtypes =3D [NotmuchDatabaseP]
> > +    _destroy.restype =3D 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