Re: [PATCH v2 10/10] ruby: new notmuch_rb_object_destroy() helper

2021-05-12 Thread Felipe Contreras 
On Wed, May 12, 2021 at 5:10 PM David Bremner  wrote:
>
> Felipe Contreras  writes:
>
> > +static inline notmuch_status_t
> > +notmuch_rb_object_destroy (VALUE rb_object, const rb_data_type_t *type)
> > +{
> > +void *nm_object;
> > +notmuch_status_t ret;
> > +
> > +Data_Get_Notmuch_Object (rb_object, type, nm_object);
> > +
> > +ret = ((notmuch_status_t (*)(void *)) type->data) (nm_object);
> > +DATA_PTR (rb_object) = NULL;
> > +
> > +return ret;
> > +}
> > +
>
> I see the benefit of making the code shorter, but I don't understand the
> new code, while I did (mostly) understand the old code. So please
> explain this in a comment as if I have no idea how ruby extensions work.

The struct used to store the types (rb_data_type_t) contains a "data"
field where we can store whatever we want. I use that field to store a
pointer to the corresponding destroy function. For example
notmuch_rb_database_type contains a pointer to
notmuch_database_destroy.

I cast that pointer as a notmuch_status_t (func*)(void *) and call
that function passing the internal object (e.g. notmuch_database_t).

Is that explanation good enough?

Cheers.

-- 
Felipe Contreras
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org

Re: [PATCH v2 10/10] ruby: new notmuch_rb_object_destroy() helper

2021-05-12 Thread David Bremner 
Felipe Contreras  writes:

> +static inline notmuch_status_t
> +notmuch_rb_object_destroy (VALUE rb_object, const rb_data_type_t *type)
> +{
> +void *nm_object;
> +notmuch_status_t ret;
> +
> +Data_Get_Notmuch_Object (rb_object, type, nm_object);
> +
> +ret = ((notmuch_status_t (*)(void *)) type->data) (nm_object);
> +DATA_PTR (rb_object) = NULL;
> +
> +return ret;
> +}
> +

I see the benefit of making the code shorter, but I don't understand the
new code, while I did (mostly) understand the old code. So please
explain this in a comment as if I have no idea how ruby extensions work.

d

___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org

[PATCH v2 10/10] ruby: new notmuch_rb_object_destroy() helper

2021-05-04 Thread Felipe Contreras 
Using the rb_data_type_t data we can call the correct notmuch destroy
function.

Signed-off-by: Felipe Contreras 
---
 bindings/ruby/database.c  |  6 +-
 bindings/ruby/defs.h  | 14 ++
 bindings/ruby/directory.c |  7 +--
 bindings/ruby/filenames.c |  7 +--
 bindings/ruby/init.c  |  1 +
 bindings/ruby/message.c   |  7 +--
 bindings/ruby/messages.c  |  7 +--
 bindings/ruby/query.c |  7 +--
 bindings/ruby/tags.c  |  7 +--
 bindings/ruby/thread.c|  7 +--
 bindings/ruby/threads.c   |  7 +--
 11 files changed, 24 insertions(+), 53 deletions(-)

diff --git a/bindings/ruby/database.c b/bindings/ruby/database.c
index 4ecc8f78..bb993d86 100644
--- a/bindings/ruby/database.c
+++ b/bindings/ruby/database.c
@@ -114,11 +114,7 @@ VALUE
 notmuch_rb_database_close (VALUE self)
 {
 notmuch_status_t ret;
-notmuch_database_t *db;
-
-Data_Get_Notmuch_Database (self, db);
-ret = notmuch_database_destroy (db);
-DATA_PTR (self) = NULL;
+ret = notmuch_rb_object_destroy (self, _rb_database_type);
 notmuch_rb_status_raise (ret);
 
 return Qnil;
diff --git a/bindings/ruby/defs.h b/bindings/ruby/defs.h
index fa7b9515..66b476e2 100644
--- a/bindings/ruby/defs.h
+++ b/bindings/ruby/defs.h
@@ -105,6 +105,20 @@ extern const rb_data_type_t notmuch_rb_tags_type;
 #define Data_Get_Notmuch_Tags(obj, ptr) \
 Data_Get_Notmuch_Object ((obj), _rb_tags_type, (ptr))
 
+static inline notmuch_status_t
+notmuch_rb_object_destroy (VALUE rb_object, const rb_data_type_t *type)
+{
+void *nm_object;
+notmuch_status_t ret;
+
+Data_Get_Notmuch_Object (rb_object, type, nm_object);
+
+ret = ((notmuch_status_t (*)(void *)) type->data) (nm_object);
+DATA_PTR (rb_object) = NULL;
+
+return ret;
+}
+
 /* status.c */
 void
 notmuch_rb_status_raise (notmuch_status_t status);
diff --git a/bindings/ruby/directory.c b/bindings/ruby/directory.c
index 17d60d1d..910f0a99 100644
--- a/bindings/ruby/directory.c
+++ b/bindings/ruby/directory.c
@@ -28,12 +28,7 @@
 VALUE
 notmuch_rb_directory_destroy (VALUE self)
 {
-notmuch_directory_t *dir;
-
-Data_Get_Notmuch_Directory (self, dir);
-
-notmuch_directory_destroy (dir);
-DATA_PTR (self) = NULL;
+notmuch_rb_object_destroy (self, _rb_directory_type);
 
 return Qnil;
 }
diff --git a/bindings/ruby/filenames.c b/bindings/ruby/filenames.c
index 656c58e6..0dec1952 100644
--- a/bindings/ruby/filenames.c
+++ b/bindings/ruby/filenames.c
@@ -28,12 +28,7 @@
 VALUE
 notmuch_rb_filenames_destroy (VALUE self)
 {
-notmuch_filenames_t *fnames;
-
-Data_Get_Notmuch_FileNames (self, fnames);
-
-notmuch_filenames_destroy (fnames);
-DATA_PTR (self) = NULL;
+notmuch_rb_object_destroy (self, _rb_filenames_type);
 
 return Qnil;
 }
diff --git a/bindings/ruby/init.c b/bindings/ruby/init.c
index a9f863eb..62515eca 100644
--- a/bindings/ruby/init.c
+++ b/bindings/ruby/init.c
@@ -54,6 +54,7 @@ const rb_data_type_t notmuch_rb_object_type = {
 const rb_data_type_t notmuch_rb_ ## id ## _type = { \
.wrap_struct_name = "notmuch_" #id, \
.parent = _rb_object_type, \
+   .data = _ ## id ## _destroy, \
 }
 
 define_type (database);
diff --git a/bindings/ruby/message.c b/bindings/ruby/message.c
index b3aed604..f45c95cc 100644
--- a/bindings/ruby/message.c
+++ b/bindings/ruby/message.c
@@ -28,12 +28,7 @@
 VALUE
 notmuch_rb_message_destroy (VALUE self)
 {
-notmuch_message_t *message;
-
-Data_Get_Notmuch_Message (self, message);
-
-notmuch_message_destroy (message);
-DATA_PTR (self) = NULL;
+notmuch_rb_object_destroy (self, _rb_message_type);
 
 return Qnil;
 }
diff --git a/bindings/ruby/messages.c b/bindings/ruby/messages.c
index e04f3af1..ca5b10d0 100644
--- a/bindings/ruby/messages.c
+++ b/bindings/ruby/messages.c
@@ -28,12 +28,7 @@
 VALUE
 notmuch_rb_messages_destroy (VALUE self)
 {
-notmuch_messages_t *messages;
-
-Data_Get_Notmuch_Messages (self, messages);
-
-notmuch_messages_destroy (messages);
-DATA_PTR (self) = NULL;
+notmuch_rb_object_destroy (self, _rb_messages_type);
 
 return Qnil;
 }
diff --git a/bindings/ruby/query.c b/bindings/ruby/query.c
index 79727d6a..3ec98c6c 100644
--- a/bindings/ruby/query.c
+++ b/bindings/ruby/query.c
@@ -28,12 +28,7 @@
 VALUE
 notmuch_rb_query_destroy (VALUE self)
 {
-notmuch_query_t *query;
-
-Data_Get_Notmuch_Query (self, query);
-
-notmuch_query_destroy (query);
-DATA_PTR (self) = NULL;
+notmuch_rb_object_destroy (self, _rb_query_type);
 
 return Qnil;
 }
diff --git a/bindings/ruby/tags.c b/bindings/ruby/tags.c
index db8b4cfc..2af85e36 100644
--- a/bindings/ruby/tags.c
+++ b/bindings/ruby/tags.c
@@ -28,12 +28,7 @@
 VALUE
 notmuch_rb_tags_destroy (VALUE self)
 {
-notmuch_tags_t *tags;
-
-Data_Get_Notmuch_Tags (self, tags);
-
-notmuch_tags_destroy (tags);
-DATA_PTR (self) = NULL;
+notmuch_rb_object_destroy (self, _rb_tags_type);