Date: Thu, 12 Apr 2012 13:02:49 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: Justus Winter <4winter@informatik.uni-hamburg.de>
Subject: Re: [PATCH 7/7] python: wrap and use notmuch_database_destroy as
	destructor
Message-ID: <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>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To:
 <1332291311-28954-8-git-send-email-4winter@informatik.uni-hamburg.de>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org
Precedence: list

Quoth Justus Winter on Mar 21 at  1:55 am:
> Adapt the go bindings to the notmuch_database_close split.

Typo.

> 
> 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/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.)

> +        '''
> +        Destroys the notmuch database.
> +        '''
> +        if self._db:
> +            self._destroy(self._db)
>              self._db = None
>  
>      def __enter__(self):