Re: [PATCH 6/7] ruby: Use notmuch_database_destroy instead of notmuch_database_close

2012-04-23 Thread Felipe Contreras
Hi,

On Sun, Apr 22, 2012 at 3:07 PM, Justus Winter
4win...@informatik.uni-hamburg.de wrote:
 Adapt the ruby bindings to the notmuch_database_close split.

 Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
 ---
  bindings/ruby/database.c |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
 index 982fd59..ba9a139 100644
 --- a/bindings/ruby/database.c
 +++ b/bindings/ruby/database.c
 @@ -110,7 +110,7 @@ notmuch_rb_database_close (VALUE self)
     notmuch_database_t *db;

     Data_Get_Notmuch_Database (self, db);
 -    notmuch_database_close (db);
 +    notmuch_database_destroy (db);
     DATA_PTR (self) = NULL;

     return Qnil;
 --

I don't think this is the right approach. If database_destroy truly
destroys the object, then we would want to do it only at garbage
collection, when it's not accessible any more. What if I want to
re-use the database from the Ruby code?

This would probably be better:

-- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -20,10 +20,16 @@

 #include defs.h

+static void
+database_free (void *p)
+{
+notmuch_database_destroy (p);
+}
+
 VALUE
 notmuch_rb_database_alloc (VALUE klass)
 {
-return Data_Wrap_Struct (klass, NULL, NULL, NULL);
+return Data_Wrap_Struct (klass, NULL, database_free, NULL);
 }

 /*

-- 
Felipe Contreras
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 6/7] ruby: Use notmuch_database_destroy instead of notmuch_database_close

2012-04-23 Thread Justus Winter
Quoting Felipe Contreras (2012-04-23 14:36:33)
 I don't think this is the right approach. If database_destroy truly
 destroys the object, then we would want to do it only at garbage
 collection, when it's not accessible any more. What if I want to
 re-use the database from the Ruby code?
 
 This would probably be better:
 
[...]

You're probably right, I don't know the ruby bindings at all, I just
wanted to preserve the old behavior. You are welcome to refine the
ruby bindings later (or maintain them, I *believe* they are
unmaintained, the last change was back in october 2011), but let's get
this patch series in first.

Cheers,
Justus
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch