[PATCH] Add method to reparent of message to the database.

[PATCH] Add method to reparent of message to the database.

[Steven Allen]

Currently, a message is owned by its iterator (messages object) and,
ultimately, its query. This unfortunately makes encapsulating query
logic difficult because there is no way to return a message after
disposing of the query used to find the message (other than looking it
up again in the database by id).

This patch adds a function `notmuch_message_own` that reparents a
message onto the database making it the user's job to destroy it.
---
 lib/message.cc |  6 ++++++
 lib/notmuch.h  | 12 ++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/lib/message.cc b/lib/message.cc
index 26b5e76..014363f 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1657,6 +1657,12 @@ notmuch_message_thaw (notmuch_message_t *message)
 }
 
 void
+notmuch_message_own (notmuch_message_t *message)
+{
+    talloc_steal(message->notmuch, message);
+}
+
+void
 notmuch_message_destroy (notmuch_message_t *message)
 {
     talloc_free (message);
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 310a8b8..c80d7aa 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1629,6 +1629,18 @@ notmuch_message_freeze (notmuch_message_t *message);
 notmuch_status_t
 notmuch_message_thaw (notmuch_message_t *message);
 
+
+/**
+ * Reparent a notmuch_message_t object onto the database.
+ *
+ * Calling this function allows a notmuch_message_t object to outlive its
+ * query. The message will automatically be reclaimed when the database is
+ * destroyed but if you want to free its memory before then, you should call
+ * notmuch_message_destroy.
+ */
+void
+notmuch_message_own (notmuch_message_t *message);
+
 /**
  * Destroy a notmuch_message_t object.
  *
-- 
2.6.2

[PATCH] Add a method to reparent threads to the database.

[Steven Allen]

This allows threads yielded from a query to outlive the query.
---
 lib/notmuch.h | 11 +++++++++++
 lib/thread.cc |  6 ++++++
 2 files changed, 17 insertions(+)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 310a8b8..9a2869b 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1188,6 +1188,17 @@ notmuch_tags_t *
 notmuch_thread_get_tags (notmuch_thread_t *thread);
 
 /**
+ * Reparent a notmuch_thread_t object onto the database.
+ *
+ * Calling this function allows a notmuch_thread_t object to outlive its
+ * query. The query will automatically be reclaimed when the database is
+ * destroyed but if you want to free its memory before then, you should call
+ * notmuch_thread_destroy.
+ */
+void
+notmuch_thread_own (notmuch_thread_t *thread);
+
+/**
  * Destroy a notmuch_thread_t object.
  */
 void
diff --git a/lib/thread.cc b/lib/thread.cc
index 0c937d7..06fa155 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -623,3 +623,9 @@ notmuch_thread_destroy (notmuch_thread_t *thread)
 {
     talloc_free (thread);
 }
+
+void
+notmuch_thread_own (notmuch_thread_t *thread)
+{
+    talloc_steal (thread->notmuch, thread);
+}
-- 
2.6.2

Re: [PATCH] Add a method to reparent threads to the database.

[Jani Nikula]

On Sat, 07 Nov 2015, Steven Allen <steven@stebalien.com> wrote:
> This allows threads yielded from a query to outlive the query.

We have a leaky abstraction in the interface. We don't properly define
object lifetimes and ownership, but we have slightly vague references to
them, and the users of the interface pretty much have to respect
them. We try to hide talloc and its concepts.

I think this patch makes the situation slightly worse.

I'd like to understand why this change is better than just holding on to
the query object. The memory saving is neglible.

BR,
Jani.


> ---
>  lib/notmuch.h | 11 +++++++++++
>  lib/thread.cc |  6 ++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 310a8b8..9a2869b 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -1188,6 +1188,17 @@ notmuch_tags_t *
>  notmuch_thread_get_tags (notmuch_thread_t *thread);
>  
>  /**
> + * Reparent a notmuch_thread_t object onto the database.
> + *
> + * Calling this function allows a notmuch_thread_t object to outlive its
> + * query. The query will automatically be reclaimed when the database is
> + * destroyed but if you want to free its memory before then, you should call
> + * notmuch_thread_destroy.
> + */
> +void
> +notmuch_thread_own (notmuch_thread_t *thread);
> +
> +/**
>   * Destroy a notmuch_thread_t object.
>   */
>  void
> diff --git a/lib/thread.cc b/lib/thread.cc
> index 0c937d7..06fa155 100644
> --- a/lib/thread.cc
> +++ b/lib/thread.cc
> @@ -623,3 +623,9 @@ notmuch_thread_destroy (notmuch_thread_t *thread)
>  {
>      talloc_free (thread);
>  }
> +
> +void
> +notmuch_thread_own (notmuch_thread_t *thread)
> +{
> +    talloc_steal (thread->notmuch, thread);
> +}
> -- 
> 2.6.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] Add a method to reparent threads to the database.

[Steven Allen]

[-- Attachment #1: Type: text/plain, Size: 2686 bytes --]

I'm writing high-level rust bindings for notmuch and would like to
enable encapsulation of query logic. That is, I'd like to be able to
write a function that performs a (set of) query(s) and returns a (or a
set of) message(s)/thread(s). I can't do this currently because the
messages/threads can't outlive the query. I could use reference counting
to store the query until all messages/threads have been freed but that
feels messy and shouldn't, strictly speaking, be necessary.

On 11-08-15, Jani Nikula wrote:
> On Sat, 07 Nov 2015, Steven Allen <steven@stebalien.com> wrote:
> > This allows threads yielded from a query to outlive the query.
> 
> We have a leaky abstraction in the interface. We don't properly define
> object lifetimes and ownership, but we have slightly vague references to
> them, and the users of the interface pretty much have to respect
> them. We try to hide talloc and its concepts.
> 
> I think this patch makes the situation slightly worse.
> 
> I'd like to understand why this change is better than just holding on to
> the query object. The memory saving is neglible.
> 
> BR,
> Jani.
> 
> 
> > ---
> >  lib/notmuch.h | 11 +++++++++++
> >  lib/thread.cc |  6 ++++++
> >  2 files changed, 17 insertions(+)
> >
> > diff --git a/lib/notmuch.h b/lib/notmuch.h
> > index 310a8b8..9a2869b 100644
> > --- a/lib/notmuch.h
> > +++ b/lib/notmuch.h
> > @@ -1188,6 +1188,17 @@ notmuch_tags_t *
> >  notmuch_thread_get_tags (notmuch_thread_t *thread);
> >  
> >  /**
> > + * Reparent a notmuch_thread_t object onto the database.
> > + *
> > + * Calling this function allows a notmuch_thread_t object to outlive its
> > + * query. The query will automatically be reclaimed when the database is
> > + * destroyed but if you want to free its memory before then, you should call
> > + * notmuch_thread_destroy.
> > + */
> > +void
> > +notmuch_thread_own (notmuch_thread_t *thread);
> > +
> > +/**
> >   * Destroy a notmuch_thread_t object.
> >   */
> >  void
> > diff --git a/lib/thread.cc b/lib/thread.cc
> > index 0c937d7..06fa155 100644
> > --- a/lib/thread.cc
> > +++ b/lib/thread.cc
> > @@ -623,3 +623,9 @@ notmuch_thread_destroy (notmuch_thread_t *thread)
> >  {
> >      talloc_free (thread);
> >  }
> > +
> > +void
> > +notmuch_thread_own (notmuch_thread_t *thread)
> > +{
> > +    talloc_steal (thread->notmuch, thread);
> > +}
> > -- 
> > 2.6.2
> >
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > https://notmuchmail.org/mailman/listinfo/notmuch

-- 
Steven Allen
(310) 433-5865
((Do Not Email <honeypot@stebalien.com>))

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Add a method to reparent threads to the database.

[David Bremner]

Steven Allen <steven@stebalien.com> writes:

> I'm writing high-level rust bindings for notmuch and would like to
> enable encapsulation of query logic. That is, I'd like to be able to
> write a function that performs a (set of) query(s) and returns a (or a
> set of) message(s)/thread(s). I can't do this currently because the
> messages/threads can't outlive the query. I could use reference counting
> to store the query until all messages/threads have been freed but that
> feels messy and shouldn't, strictly speaking, be necessary.

It probably wouldn't be as efficient, but it might be a cleaner API to
have a way to copy a message or thread into a malloced (or maybe user-provided
buffer).