[PATCH 1/7] Split notmuch_database_close into two functions

[David Bremner]

Justus Winter <4winter at informatik.uni-hamburg.de> writes:

> Formerly notmuch_database_close closed the xapian database and
> destroyed the talloc structure associated with the notmuch database
> object. Split notmuch_database_close into notmuch_database_close and
> notmuch_database_destroy.

Series pushed.

d

Re: [PATCH 1/7] Split notmuch_database_close into two functions

[David Bremner]

Justus Winter 4win...@informatik.uni-hamburg.de writes:

 Formerly notmuch_database_close closed the xapian database and
 destroyed the talloc structure associated with the notmuch database
 object. Split notmuch_database_close into notmuch_database_close and
 notmuch_database_destroy.

Series pushed.

d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 1/7] Split notmuch_database_close into two functions

[Justus Winter]

Formerly notmuch_database_close closed the xapian database and
destroyed the talloc structure associated with the notmuch database
object. Split notmuch_database_close into notmuch_database_close and
notmuch_database_destroy.

This makes it possible for long running programs to close the xapian
database and thus release the lock associated with it without
destroying the data structures obtained from it.

This also makes the api more consistent since every other data
structure has a destructor function.

The comments in notmuch.h are a courtesy of Austin Clements.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 lib/database.cc |   14 --
 lib/notmuch.h   |   22 ++
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 16c4354..2fefcad 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
 "   read-write mode.\n",
 notmuch_path, version, NOTMUCH_DATABASE_VERSION);
notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
goto DONE;
}
@@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
 } catch (const Xapian::Error ) {
fprintf (stderr, "A Xapian exception occurred opening database: %s\n",
 error.get_msg().c_str());
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
 }

@@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
 }

 delete notmuch->term_gen;
+notmuch->term_gen = NULL;
 delete notmuch->query_parser;
+notmuch->query_parser = NULL;
 delete notmuch->xapian_db;
+notmuch->xapian_db = NULL;
 delete notmuch->value_range_processor;
+notmuch->value_range_processor = NULL;
+}
+
+void
+notmuch_database_destroy (notmuch_database_t *notmuch)
+{
+notmuch_database_close (notmuch);
 talloc_free (notmuch);
 }

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 673c423..7d9e092 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
  *
  * After a successful call to notmuch_database_create, the returned
  * database will be open so the caller should call
- * notmuch_database_close when finished with it.
+ * notmuch_database_destroy when finished with it.
  *
  * The database will not yet have any data in it
  * (notmuch_database_create itself is a very cheap function). Messages
@@ -165,7 +165,7 @@ typedef enum {
  * An existing notmuch database can be identified by the presence of a
  * directory named ".notmuch" below 'path'.
  *
- * The caller should call notmuch_database_close when finished with
+ * The caller should call notmuch_database_destroy when finished with
  * this database.
  *
  * In case of any failure, this function returns NULL, (after printing
@@ -175,11 +175,25 @@ notmuch_database_t *
 notmuch_database_open (const char *path,
   notmuch_database_mode_t mode);

-/* Close the given notmuch database, freeing all associated
- * resources. See notmuch_database_open. */
+/* Close the given notmuch database.
+ *
+ * After notmuch_database_close has been called, calls to other
+ * functions on objects derived from this database may either behave
+ * as if the database had not been closed (e.g., if the required data
+ * has been cached) or may fail with a
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION.
+ *
+ * notmuch_database_close can be called multiple times.  Later calls
+ * have no effect.
+ */
 void
 notmuch_database_close (notmuch_database_t *database);

+/* Destroy the notmuch database, closing it if necessary and freeing
+* all associated resources. */
+void
+notmuch_database_destroy (notmuch_database_t *database);
+
 /* Return the database path of the given database.
  *
  * The return value is a string owned by notmuch so should not be
-- 
1.7.10

[PATCH 1/7] Split notmuch_database_close into two functions

[Austin Clements]

LGTM.

On Wed, 25 Apr 2012, Justus Winter <4winter at informatik.uni-hamburg.de> wrote:
> Formerly notmuch_database_close closed the xapian database and
> destroyed the talloc structure associated with the notmuch database
> object. Split notmuch_database_close into notmuch_database_close and
> notmuch_database_destroy.
>
> This makes it possible for long running programs to close the xapian
> database and thus release the lock associated with it without
> destroying the data structures obtained from it.
>
> This also makes the api more consistent since every other data
> structure has a destructor function.
>
> The comments in notmuch.h are a courtesy of Austin Clements.
>
> Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>

[PATCH 1/7] Split notmuch_database_close into two functions

[Justus Winter]

Formerly notmuch_database_close closed the xapian database and
destroyed the talloc structure associated with the notmuch database
object. Split notmuch_database_close into notmuch_database_close and
notmuch_database_destroy.

This makes it possible for long running programs to close the xapian
database and thus release the lock associated with it without
destroying the data structures obtained from it.

This also makes the api more consistent since every other data
structure has a destructor function.

The comments in notmuch.h are a courtesy of Austin Clements.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 lib/database.cc |   14 --
 lib/notmuch.h   |   22 ++
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 16c4354..2fefcad 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
read-write mode.\n,
 notmuch_path, version, NOTMUCH_DATABASE_VERSION);
notmuch-mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
goto DONE;
}
@@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
 } catch (const Xapian::Error error) {
fprintf (stderr, A Xapian exception occurred opening database: %s\n,
 error.get_msg().c_str());
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
 }
 
@@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
 }
 
 delete notmuch-term_gen;
+notmuch-term_gen = NULL;
 delete notmuch-query_parser;
+notmuch-query_parser = NULL;
 delete notmuch-xapian_db;
+notmuch-xapian_db = NULL;
 delete notmuch-value_range_processor;
+notmuch-value_range_processor = NULL;
+}
+
+void
+notmuch_database_destroy (notmuch_database_t *notmuch)
+{
+notmuch_database_close (notmuch);
 talloc_free (notmuch);
 }
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 673c423..7d9e092 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
  *
  * After a successful call to notmuch_database_create, the returned
  * database will be open so the caller should call
- * notmuch_database_close when finished with it.
+ * notmuch_database_destroy when finished with it.
  *
  * The database will not yet have any data in it
  * (notmuch_database_create itself is a very cheap function). Messages
@@ -165,7 +165,7 @@ typedef enum {
  * An existing notmuch database can be identified by the presence of a
  * directory named .notmuch below 'path'.
  *
- * The caller should call notmuch_database_close when finished with
+ * The caller should call notmuch_database_destroy when finished with
  * this database.
  *
  * In case of any failure, this function returns NULL, (after printing
@@ -175,11 +175,25 @@ notmuch_database_t *
 notmuch_database_open (const char *path,
   notmuch_database_mode_t mode);
 
-/* Close the given notmuch database, freeing all associated
- * resources. See notmuch_database_open. */
+/* Close the given notmuch database.
+ *
+ * After notmuch_database_close has been called, calls to other
+ * functions on objects derived from this database may either behave
+ * as if the database had not been closed (e.g., if the required data
+ * has been cached) or may fail with a
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION.
+ *
+ * notmuch_database_close can be called multiple times.  Later calls
+ * have no effect.
+ */
 void
 notmuch_database_close (notmuch_database_t *database);
 
+/* Destroy the notmuch database, closing it if necessary and freeing
+* all associated resources. */
+void
+notmuch_database_destroy (notmuch_database_t *database);
+
 /* Return the database path of the given database.
  *
  * The return value is a string owned by notmuch so should not be
-- 
1.7.10

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

Re: [PATCH 1/7] Split notmuch_database_close into two functions

[Austin Clements]

LGTM.

On Wed, 25 Apr 2012, Justus Winter 4win...@informatik.uni-hamburg.de wrote:
 Formerly notmuch_database_close closed the xapian database and
 destroyed the talloc structure associated with the notmuch database
 object. Split notmuch_database_close into notmuch_database_close and
 notmuch_database_destroy.

 This makes it possible for long running programs to close the xapian
 database and thus release the lock associated with it without
 destroying the data structures obtained from it.

 This also makes the api more consistent since every other data
 structure has a destructor function.

 The comments in notmuch.h are a courtesy of Austin Clements.

 Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 1/7] Split notmuch_database_close into two functions

[Justus Winter]

Formerly notmuch_database_close closed the xapian database and
destroyed the talloc structure associated with the notmuch database
object. Split notmuch_database_close into notmuch_database_close and
notmuch_database_destroy.

This makes it possible for long running programs to close the xapian
database and thus release the lock associated with it without
destroying the data structures obtained from it.

This also makes the api more consistent since every other data
structure has a destructor function.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 lib/database.cc |   14 --
 lib/notmuch.h   |   15 +++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 16c4354..2fefcad 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
 "   read-write mode.\n",
 notmuch_path, version, NOTMUCH_DATABASE_VERSION);
notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
goto DONE;
}
@@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
 } catch (const Xapian::Error ) {
fprintf (stderr, "A Xapian exception occurred opening database: %s\n",
 error.get_msg().c_str());
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
 }

@@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
 }

 delete notmuch->term_gen;
+notmuch->term_gen = NULL;
 delete notmuch->query_parser;
+notmuch->query_parser = NULL;
 delete notmuch->xapian_db;
+notmuch->xapian_db = NULL;
 delete notmuch->value_range_processor;
+notmuch->value_range_processor = NULL;
+}
+
+void
+notmuch_database_destroy (notmuch_database_t *notmuch)
+{
+notmuch_database_close (notmuch);
 talloc_free (notmuch);
 }

diff --git a/lib/notmuch.h b/lib/notmuch.h
index 673c423..84c9265 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
  *
  * After a successful call to notmuch_database_create, the returned
  * database will be open so the caller should call
- * notmuch_database_close when finished with it.
+ * notmuch_database_destroy when finished with it.
  *
  * The database will not yet have any data in it
  * (notmuch_database_create itself is a very cheap function). Messages
@@ -165,7 +165,7 @@ typedef enum {
  * An existing notmuch database can be identified by the presence of a
  * directory named ".notmuch" below 'path'.
  *
- * The caller should call notmuch_database_close when finished with
+ * The caller should call notmuch_database_destroy when finished with
  * this database.
  *
  * In case of any failure, this function returns NULL, (after printing
@@ -175,11 +175,18 @@ notmuch_database_t *
 notmuch_database_open (const char *path,
   notmuch_database_mode_t mode);

-/* Close the given notmuch database, freeing all associated
- * resources. See notmuch_database_open. */
+/* Close the given notmuch database.
+ *
+ * This function is called by notmuch_database_destroy and can be
+ * called multiple times. */
 void
 notmuch_database_close (notmuch_database_t *database);

+/* Destroy the notmuch database freeing all associated
+ * resources */
+void
+notmuch_database_destroy (notmuch_database_t *database);
+
 /* Return the database path of the given database.
  *
  * The return value is a string owned by notmuch so should not be
-- 
1.7.10

[PATCH 1/7] Split notmuch_database_close into two functions

[Austin Clements]

On Sun, 22 Apr 2012, Justus Winter <4winter at informatik.uni-hamburg.de> wrote:
> Formerly notmuch_database_close closed the xapian database and
> destroyed the talloc structure associated with the notmuch database
> object. Split notmuch_database_close into notmuch_database_close and
> notmuch_database_destroy.
>
> This makes it possible for long running programs to close the xapian
> database and thus release the lock associated with it without
> destroying the data structures obtained from it.
>
> This also makes the api more consistent since every other data
> structure has a destructor function.
>
> Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>

Other than my comments comment, this series LGTM.  I think adding those
comments should also address Felipe's question.

[PATCH 1/7] Split notmuch_database_close into two functions

[Austin Clements]

Quoth Justus Winter on Apr 22 at  2:07 pm:
> Formerly notmuch_database_close closed the xapian database and
> destroyed the talloc structure associated with the notmuch database
> object. Split notmuch_database_close into notmuch_database_close and
> notmuch_database_destroy.
> 
> This makes it possible for long running programs to close the xapian
> database and thus release the lock associated with it without
> destroying the data structures obtained from it.
> 
> This also makes the api more consistent since every other data
> structure has a destructor function.
> 
> Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
> ---
>  lib/database.cc |   14 --
>  lib/notmuch.h   |   15 +++
>  2 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/database.cc b/lib/database.cc
> index 16c4354..2fefcad 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
>"   read-write mode.\n",
>notmuch_path, version, NOTMUCH_DATABASE_VERSION);
>   notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
> - notmuch_database_close (notmuch);
> + notmuch_database_destroy (notmuch);
>   notmuch = NULL;
>   goto DONE;
>   }
> @@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
>  } catch (const Xapian::Error ) {
>   fprintf (stderr, "A Xapian exception occurred opening database: %s\n",
>error.get_msg().c_str());
> - notmuch_database_close (notmuch);
> + notmuch_database_destroy (notmuch);
>   notmuch = NULL;
>  }
>  
> @@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
>  }
>  
>  delete notmuch->term_gen;
> +notmuch->term_gen = NULL;
>  delete notmuch->query_parser;
> +notmuch->query_parser = NULL;
>  delete notmuch->xapian_db;
> +notmuch->xapian_db = NULL;
>  delete notmuch->value_range_processor;
> +notmuch->value_range_processor = NULL;
> +}
> +
> +void
> +notmuch_database_destroy (notmuch_database_t *notmuch)
> +{
> +notmuch_database_close (notmuch);
>  talloc_free (notmuch);
>  }
>  
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 673c423..84c9265 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
>   *
>   * After a successful call to notmuch_database_create, the returned
>   * database will be open so the caller should call
> - * notmuch_database_close when finished with it.
> + * notmuch_database_destroy when finished with it.
>   *
>   * The database will not yet have any data in it
>   * (notmuch_database_create itself is a very cheap function). Messages
> @@ -165,7 +165,7 @@ typedef enum {
>   * An existing notmuch database can be identified by the presence of a
>   * directory named ".notmuch" below 'path'.
>   *
> - * The caller should call notmuch_database_close when finished with
> + * The caller should call notmuch_database_destroy when finished with
>   * this database.
>   *
>   * In case of any failure, this function returns NULL, (after printing
> @@ -175,11 +175,18 @@ notmuch_database_t *
>  notmuch_database_open (const char *path,
>  notmuch_database_mode_t mode);
>  
> -/* Close the given notmuch database, freeing all associated
> - * resources. See notmuch_database_open. */
> +/* Close the given notmuch database.
> + *
> + * This function is called by notmuch_database_destroy and can be
> + * called multiple times. */

This needs a comment explaining the implications of closing the
database.  Perhaps something like this (modeled on
Xapian::Database::close's documentation)

/* Close the given notmuch database.
 *
 * After notmuch_database_close has been called, calls to other
 * functions on objects derived from this database may either behave
 * as if the database had not been closed (e.g., if the required data
 * has been cached) or may fail with a
 * NOTMUCH_STATUS_XAPIAN_EXCEPTION.
 *
 * notmuch_database_close can be called multiple times.  Later calls
 * have no affect. 
 */

>  void
>  notmuch_database_close (notmuch_database_t *database);
>  
> +/* Destroy the notmuch database freeing all associated
> + * resources */

This should mention that this also closes the database (currently you
mention this in the doc for notmuch_database_close, but it's a reverse
reference there; that could probably even be omitted).  Perhaps

/* Destroy the notmuch database, closing it if necessary and freeing
 * all associated resources. */

> +void
> +notmuch_database_destroy (notmuch_database_t *database);
> +
>  /* Return the database path of the given database.
>   *
>   * The return value is a string owned by notmuch so should not be

[PATCH 1/7] Split notmuch_database_close into two functions

[Justus Winter]

Formerly notmuch_database_close closed the xapian database and
destroyed the talloc structure associated with the notmuch database
object. Split notmuch_database_close into notmuch_database_close and
notmuch_database_destroy.

This makes it possible for long running programs to close the xapian
database and thus release the lock associated with it without
destroying the data structures obtained from it.

This also makes the api more consistent since every other data
structure has a destructor function.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 lib/database.cc |   14 --
 lib/notmuch.h   |   15 +++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 16c4354..2fefcad 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
read-write mode.\n,
 notmuch_path, version, NOTMUCH_DATABASE_VERSION);
notmuch-mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
goto DONE;
}
@@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
 } catch (const Xapian::Error error) {
fprintf (stderr, A Xapian exception occurred opening database: %s\n,
 error.get_msg().c_str());
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
 }
 
@@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
 }
 
 delete notmuch-term_gen;
+notmuch-term_gen = NULL;
 delete notmuch-query_parser;
+notmuch-query_parser = NULL;
 delete notmuch-xapian_db;
+notmuch-xapian_db = NULL;
 delete notmuch-value_range_processor;
+notmuch-value_range_processor = NULL;
+}
+
+void
+notmuch_database_destroy (notmuch_database_t *notmuch)
+{
+notmuch_database_close (notmuch);
 talloc_free (notmuch);
 }
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 673c423..84c9265 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
  *
  * After a successful call to notmuch_database_create, the returned
  * database will be open so the caller should call
- * notmuch_database_close when finished with it.
+ * notmuch_database_destroy when finished with it.
  *
  * The database will not yet have any data in it
  * (notmuch_database_create itself is a very cheap function). Messages
@@ -165,7 +165,7 @@ typedef enum {
  * An existing notmuch database can be identified by the presence of a
  * directory named .notmuch below 'path'.
  *
- * The caller should call notmuch_database_close when finished with
+ * The caller should call notmuch_database_destroy when finished with
  * this database.
  *
  * In case of any failure, this function returns NULL, (after printing
@@ -175,11 +175,18 @@ notmuch_database_t *
 notmuch_database_open (const char *path,
   notmuch_database_mode_t mode);
 
-/* Close the given notmuch database, freeing all associated
- * resources. See notmuch_database_open. */
+/* Close the given notmuch database.
+ *
+ * This function is called by notmuch_database_destroy and can be
+ * called multiple times. */
 void
 notmuch_database_close (notmuch_database_t *database);
 
+/* Destroy the notmuch database freeing all associated
+ * resources */
+void
+notmuch_database_destroy (notmuch_database_t *database);
+
 /* Return the database path of the given database.
  *
  * The return value is a string owned by notmuch so should not be
-- 
1.7.10

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

Re: [PATCH 1/7] Split notmuch_database_close into two functions

[Austin Clements]

Quoth Justus Winter on Apr 22 at  2:07 pm:
 Formerly notmuch_database_close closed the xapian database and
 destroyed the talloc structure associated with the notmuch database
 object. Split notmuch_database_close into notmuch_database_close and
 notmuch_database_destroy.
 
 This makes it possible for long running programs to close the xapian
 database and thus release the lock associated with it without
 destroying the data structures obtained from it.
 
 This also makes the api more consistent since every other data
 structure has a destructor function.
 
 Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
 ---
  lib/database.cc |   14 --
  lib/notmuch.h   |   15 +++
  2 files changed, 23 insertions(+), 6 deletions(-)
 
 diff --git a/lib/database.cc b/lib/database.cc
 index 16c4354..2fefcad 100644
 --- a/lib/database.cc
 +++ b/lib/database.cc
 @@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
   read-write mode.\n,
notmuch_path, version, NOTMUCH_DATABASE_VERSION);
   notmuch-mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
 - notmuch_database_close (notmuch);
 + notmuch_database_destroy (notmuch);
   notmuch = NULL;
   goto DONE;
   }
 @@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
  } catch (const Xapian::Error error) {
   fprintf (stderr, A Xapian exception occurred opening database: %s\n,
error.get_msg().c_str());
 - notmuch_database_close (notmuch);
 + notmuch_database_destroy (notmuch);
   notmuch = NULL;
  }
  
 @@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
  }
  
  delete notmuch-term_gen;
 +notmuch-term_gen = NULL;
  delete notmuch-query_parser;
 +notmuch-query_parser = NULL;
  delete notmuch-xapian_db;
 +notmuch-xapian_db = NULL;
  delete notmuch-value_range_processor;
 +notmuch-value_range_processor = NULL;
 +}
 +
 +void
 +notmuch_database_destroy (notmuch_database_t *notmuch)
 +{
 +notmuch_database_close (notmuch);
  talloc_free (notmuch);
  }
  
 diff --git a/lib/notmuch.h b/lib/notmuch.h
 index 673c423..84c9265 100644
 --- a/lib/notmuch.h
 +++ b/lib/notmuch.h
 @@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
   *
   * After a successful call to notmuch_database_create, the returned
   * database will be open so the caller should call
 - * notmuch_database_close when finished with it.
 + * notmuch_database_destroy when finished with it.
   *
   * The database will not yet have any data in it
   * (notmuch_database_create itself is a very cheap function). Messages
 @@ -165,7 +165,7 @@ typedef enum {
   * An existing notmuch database can be identified by the presence of a
   * directory named .notmuch below 'path'.
   *
 - * The caller should call notmuch_database_close when finished with
 + * The caller should call notmuch_database_destroy when finished with
   * this database.
   *
   * In case of any failure, this function returns NULL, (after printing
 @@ -175,11 +175,18 @@ notmuch_database_t *
  notmuch_database_open (const char *path,
  notmuch_database_mode_t mode);
  
 -/* Close the given notmuch database, freeing all associated
 - * resources. See notmuch_database_open. */
 +/* Close the given notmuch database.
 + *
 + * This function is called by notmuch_database_destroy and can be
 + * called multiple times. */

This needs a comment explaining the implications of closing the
database.  Perhaps something like this (modeled on
Xapian::Database::close's documentation)

/* Close the given notmuch database.
 *
 * After notmuch_database_close has been called, calls to other
 * functions on objects derived from this database may either behave
 * as if the database had not been closed (e.g., if the required data
 * has been cached) or may fail with a
 * NOTMUCH_STATUS_XAPIAN_EXCEPTION.
 *
 * notmuch_database_close can be called multiple times.  Later calls
 * have no affect. 
 */

  void
  notmuch_database_close (notmuch_database_t *database);
  
 +/* Destroy the notmuch database freeing all associated
 + * resources */

This should mention that this also closes the database (currently you
mention this in the doc for notmuch_database_close, but it's a reverse
reference there; that could probably even be omitted).  Perhaps

/* Destroy the notmuch database, closing it if necessary and freeing
 * all associated resources. */

 +void
 +notmuch_database_destroy (notmuch_database_t *database);
 +
  /* Return the database path of the given database.
   *
   * The return value is a string owned by notmuch so should not be
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 1/7] Split notmuch_database_close into two functions

[Mark Walters]

On Mon, 16 Apr 2012, Justus Winter <4winter at informatik.uni-hamburg.de> wrote:
> Quoting Mark Walters (2012-03-31 19:17:15)
>> Secondly, I think the patch series could be made clearer and easier to
>> review. If you do it in three steps
>> 
>> 1) change of notmuch_database_close to notmuch_database_destroy (just
>>the function name change)
>> 2) split the new notmuch_database_destroy into two as in the current
>>first patch
>> 3) Make any changes (if there are any) of notmuch_database_destroy to
>>notmuch_database_close.
>> 
>> The advantage is that the first change is easy to test (essentially does
>> it build) and then changes from notmuch_database_destroy to
>> notmuch_database_close in step 3 are explicit rather than the current
>> situation where we need to grep the code to see if some instances of
>> notmuch_database_close were not changed to notmuch_database_destroy.
>
> I don't buy it. The patch series first touches the library and
> documentation and the lib compiles fine. The next patch updates the
> cli tools, all of them compile fine afterwards.
>
> Every patch addresses the issue component wise, this seems rather
> natural for me.

I will try an explain my concern better. I assume that the patch
actually introduces a functional change : that is something somewhere in
the code calls the new notmuch_database_close instead of
notmuch_database_destroy [1]. In your current patch series someone
reading the patches alone can't see the functional change: it comes from
the occurrences of notmuch_database_close that you *don't* change to
notmuch_database_destroy.

Indeed, if the only change is to allow out-of-tree code access to the
new notmuch_database_close function then doing the patch series as

rename notmuch_database_close to notmuch_database_destroy

Then split notmuch_database_destroy make it clearer. (And if the code
compiles after step 1 then I know *all* occurrences of
notmuch_database_close have been changed to notmuch_database_destroy).


Best wishes

Mark

[1] Apart, of course, from notmuch_database_destroy.

[PATCH 1/7] Split notmuch_database_close into two functions

[Justus Winter]

Quoting Mark Walters (2012-03-31 19:17:15)
> Secondly, I think the patch series could be made clearer and easier to
> review. If you do it in three steps
> 
> 1) change of notmuch_database_close to notmuch_database_destroy (just
>the function name change)
> 2) split the new notmuch_database_destroy into two as in the current
>first patch
> 3) Make any changes (if there are any) of notmuch_database_destroy to
>notmuch_database_close.
> 
> The advantage is that the first change is easy to test (essentially does
> it build) and then changes from notmuch_database_destroy to
> notmuch_database_close in step 3 are explicit rather than the current
> situation where we need to grep the code to see if some instances of
> notmuch_database_close were not changed to notmuch_database_destroy.

I don't buy it. The patch series first touches the library and
documentation and the lib compiles fine. The next patch updates the
cli tools, all of them compile fine afterwards.

Every patch addresses the issue component wise, this seems rather
natural for me.

Cheers,
Justus

Re: [PATCH 1/7] Split notmuch_database_close into two functions

[Mark Walters]

On Mon, 16 Apr 2012, Justus Winter 4win...@informatik.uni-hamburg.de wrote:
 Quoting Mark Walters (2012-03-31 19:17:15)
 Secondly, I think the patch series could be made clearer and easier to
 review. If you do it in three steps
 
 1) change of notmuch_database_close to notmuch_database_destroy (just
the function name change)
 2) split the new notmuch_database_destroy into two as in the current
first patch
 3) Make any changes (if there are any) of notmuch_database_destroy to
notmuch_database_close.
 
 The advantage is that the first change is easy to test (essentially does
 it build) and then changes from notmuch_database_destroy to
 notmuch_database_close in step 3 are explicit rather than the current
 situation where we need to grep the code to see if some instances of
 notmuch_database_close were not changed to notmuch_database_destroy.

 I don't buy it. The patch series first touches the library and
 documentation and the lib compiles fine. The next patch updates the
 cli tools, all of them compile fine afterwards.

 Every patch addresses the issue component wise, this seems rather
 natural for me.

I will try an explain my concern better. I assume that the patch
actually introduces a functional change : that is something somewhere in
the code calls the new notmuch_database_close instead of
notmuch_database_destroy [1]. In your current patch series someone
reading the patches alone can't see the functional change: it comes from
the occurrences of notmuch_database_close that you *don't* change to
notmuch_database_destroy.

Indeed, if the only change is to allow out-of-tree code access to the
new notmuch_database_close function then doing the patch series as

rename notmuch_database_close to notmuch_database_destroy

Then split notmuch_database_destroy make it clearer. (And if the code
compiles after step 1 then I know *all* occurrences of
notmuch_database_close have been changed to notmuch_database_destroy).


Best wishes

Mark

[1] Apart, of course, from notmuch_database_destroy.

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

[PATCH 1/7] Split notmuch_database_close into two functions

[Mark Walters]

Justus Winter <4winter at informatik.uni-hamburg.de> writes:

> Formerly notmuch_database_close closed the xapian database and
> destroyed the talloc structure associated with the notmuch database
> object. Split notmuch_database_close into notmuch_database_close and
> notmuch_database_destroy.
>
> This makes it possible for long running programs to close the xapian
> database and thus release the lock associated with it without
> destroying the data structures obtained from it.
>
> This also makes the api more consistent since every other data
> structure has a destructor function.

I like the idea of this series but have two queries before reviewing it.

The first is a concern that if we change the library functions we should
update the library version otherwise out of tree users won't know which
to call. (I don't actually know how versioning is done but I think we
should at least be able to make new out-of-tree code cope with the old
or new version). It might be worth keeping notmuch_database_close as it
is for now and adding something like notmuch_database_weak_close with
the new functionality. Then whenever the library version is next going
to get bumped we could move to the destroy/close nomenclature.

Secondly, I think the patch series could be made clearer and easier to
review. If you do it in three steps

1) change of notmuch_database_close to notmuch_database_destroy (just
   the function name change)
2) split the new notmuch_database_destroy into two as in the current
   first patch
3) Make any changes (if there are any) of notmuch_database_destroy to
   notmuch_database_close.

The advantage is that the first change is easy to test (essentially does
it build) and then changes from notmuch_database_destroy to
notmuch_database_close in step 3 are explicit rather than the current
situation where we need to grep the code to see if some instances of
notmuch_database_close were not changed to notmuch_database_destroy.

Of course if the decision is to go via the weak_close version then you
just need to do the analogues of 2 and 3.

Best wishes

Mark

>
> Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
> ---
>  lib/database.cc |   14 --
>  lib/notmuch.h   |   15 +++
>  2 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 16c4354..2fefcad 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
>"   read-write mode.\n",
>notmuch_path, version, NOTMUCH_DATABASE_VERSION);
>   notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
> - notmuch_database_close (notmuch);
> + notmuch_database_destroy (notmuch);
>   notmuch = NULL;
>   goto DONE;
>   }
> @@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
>  } catch (const Xapian::Error ) {
>   fprintf (stderr, "A Xapian exception occurred opening database: %s\n",
>error.get_msg().c_str());
> - notmuch_database_close (notmuch);
> + notmuch_database_destroy (notmuch);
>   notmuch = NULL;
>  }
>  
> @@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
>  }
>  
>  delete notmuch->term_gen;
> +notmuch->term_gen = NULL;
>  delete notmuch->query_parser;
> +notmuch->query_parser = NULL;
>  delete notmuch->xapian_db;
> +notmuch->xapian_db = NULL;
>  delete notmuch->value_range_processor;
> +notmuch->value_range_processor = NULL;
> +}
> +
> +void
> +notmuch_database_destroy (notmuch_database_t *notmuch)
> +{
> +notmuch_database_close (notmuch);
>  talloc_free (notmuch);
>  }
>  
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index babd208..6114f36 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
>   *
>   * After a successful call to notmuch_database_create, the returned
>   * database will be open so the caller should call
> - * notmuch_database_close when finished with it.
> + * notmuch_database_destroy when finished with it.
>   *
>   * The database will not yet have any data in it
>   * (notmuch_database_create itself is a very cheap function). Messages
> @@ -165,7 +165,7 @@ typedef enum {
>   * An existing notmuch database can be identified by the presence of a
>   * directory named ".notmuch" below 'path'.
>   *
> - * The caller should call notmuch_database_close when finished with
> + * The caller should call notmuch_database_destroy when finished with
>   * this database.
>   *
>   * In case of any failure, this function returns NULL, (after printing
> @@ -175,11 +175,18 @@ notmuch_database_t *
>  notmuch_database_open (const char *path,
>  notmuch_database_mode_t mode);
>  
> -/* Close the given notmuch database, freeing all associated
> - * resources. See notmuch_database_open. */

[PATCH 1/7] Split notmuch_database_close into two functions

[David Bremner]

Mark Walters  writes:

> The first is a concern that if we change the library functions we should
> update the library version otherwise out of tree users won't know which
> to call. 

This is not such a big deal. We can update the SONAME of the library so
that old code will continute to dynamically link to the old version of
the library. Nothing [1] will magically make the old code work with the
new library if we change the API; that is just the way things go,
occasionally APIs and/or ABIs change.

[1] Well maybe symbol versioning. I don't fully understand that, but I
suspect it is more trouble than it is worth.

Re: [PATCH 1/7] Split notmuch_database_close into two functions

[Mark Walters]

Justus Winter 4win...@informatik.uni-hamburg.de writes:

 Formerly notmuch_database_close closed the xapian database and
 destroyed the talloc structure associated with the notmuch database
 object. Split notmuch_database_close into notmuch_database_close and
 notmuch_database_destroy.

 This makes it possible for long running programs to close the xapian
 database and thus release the lock associated with it without
 destroying the data structures obtained from it.

 This also makes the api more consistent since every other data
 structure has a destructor function.

I like the idea of this series but have two queries before reviewing it.

The first is a concern that if we change the library functions we should
update the library version otherwise out of tree users won't know which
to call. (I don't actually know how versioning is done but I think we
should at least be able to make new out-of-tree code cope with the old
or new version). It might be worth keeping notmuch_database_close as it
is for now and adding something like notmuch_database_weak_close with
the new functionality. Then whenever the library version is next going
to get bumped we could move to the destroy/close nomenclature.

Secondly, I think the patch series could be made clearer and easier to
review. If you do it in three steps

1) change of notmuch_database_close to notmuch_database_destroy (just
   the function name change)
2) split the new notmuch_database_destroy into two as in the current
   first patch
3) Make any changes (if there are any) of notmuch_database_destroy to
   notmuch_database_close.

The advantage is that the first change is easy to test (essentially does
it build) and then changes from notmuch_database_destroy to
notmuch_database_close in step 3 are explicit rather than the current
situation where we need to grep the code to see if some instances of
notmuch_database_close were not changed to notmuch_database_destroy.

Of course if the decision is to go via the weak_close version then you
just need to do the analogues of 2 and 3.

Best wishes

Mark


 Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
 ---
  lib/database.cc |   14 --
  lib/notmuch.h   |   15 +++
  2 files changed, 23 insertions(+), 6 deletions(-)

 diff --git a/lib/database.cc b/lib/database.cc
 index 16c4354..2fefcad 100644
 --- a/lib/database.cc
 +++ b/lib/database.cc
 @@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
   read-write mode.\n,
notmuch_path, version, NOTMUCH_DATABASE_VERSION);
   notmuch-mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
 - notmuch_database_close (notmuch);
 + notmuch_database_destroy (notmuch);
   notmuch = NULL;
   goto DONE;
   }
 @@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
  } catch (const Xapian::Error error) {
   fprintf (stderr, A Xapian exception occurred opening database: %s\n,
error.get_msg().c_str());
 - notmuch_database_close (notmuch);
 + notmuch_database_destroy (notmuch);
   notmuch = NULL;
  }
  
 @@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
  }
  
  delete notmuch-term_gen;
 +notmuch-term_gen = NULL;
  delete notmuch-query_parser;
 +notmuch-query_parser = NULL;
  delete notmuch-xapian_db;
 +notmuch-xapian_db = NULL;
  delete notmuch-value_range_processor;
 +notmuch-value_range_processor = NULL;
 +}
 +
 +void
 +notmuch_database_destroy (notmuch_database_t *notmuch)
 +{
 +notmuch_database_close (notmuch);
  talloc_free (notmuch);
  }
  
 diff --git a/lib/notmuch.h b/lib/notmuch.h
 index babd208..6114f36 100644
 --- a/lib/notmuch.h
 +++ b/lib/notmuch.h
 @@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
   *
   * After a successful call to notmuch_database_create, the returned
   * database will be open so the caller should call
 - * notmuch_database_close when finished with it.
 + * notmuch_database_destroy when finished with it.
   *
   * The database will not yet have any data in it
   * (notmuch_database_create itself is a very cheap function). Messages
 @@ -165,7 +165,7 @@ typedef enum {
   * An existing notmuch database can be identified by the presence of a
   * directory named .notmuch below 'path'.
   *
 - * The caller should call notmuch_database_close when finished with
 + * The caller should call notmuch_database_destroy when finished with
   * this database.
   *
   * In case of any failure, this function returns NULL, (after printing
 @@ -175,11 +175,18 @@ notmuch_database_t *
  notmuch_database_open (const char *path,
  notmuch_database_mode_t mode);
  
 -/* Close the given notmuch database, freeing all associated
 - * resources. See notmuch_database_open. */
 +/* Close the given notmuch database.
 + *
 + * This function is called by notmuch_database_destroyed and 

Re: [PATCH 1/7] Split notmuch_database_close into two functions

[David Bremner]

Mark Walters markwalters1...@gmail.com writes:

 The first is a concern that if we change the library functions we should
 update the library version otherwise out of tree users won't know which
 to call. 

This is not such a big deal. We can update the SONAME of the library so
that old code will continute to dynamically link to the old version of
the library. Nothing [1] will magically make the old code work with the
new library if we change the API; that is just the way things go,
occasionally APIs and/or ABIs change.

[1] Well maybe symbol versioning. I don't fully understand that, but I
suspect it is more trouble than it is worth.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 1/7] Split notmuch_database_close into two functions

[Justus Winter]

Formerly notmuch_database_close closed the xapian database and
destroyed the talloc structure associated with the notmuch database
object. Split notmuch_database_close into notmuch_database_close and
notmuch_database_destroy.

This makes it possible for long running programs to close the xapian
database and thus release the lock associated with it without
destroying the data structures obtained from it.

This also makes the api more consistent since every other data
structure has a destructor function.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 lib/database.cc |   14 --
 lib/notmuch.h   |   15 +++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 16c4354..2fefcad 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
 "   read-write mode.\n",
 notmuch_path, version, NOTMUCH_DATABASE_VERSION);
notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
goto DONE;
}
@@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
 } catch (const Xapian::Error ) {
fprintf (stderr, "A Xapian exception occurred opening database: %s\n",
 error.get_msg().c_str());
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
 }

@@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
 }

 delete notmuch->term_gen;
+notmuch->term_gen = NULL;
 delete notmuch->query_parser;
+notmuch->query_parser = NULL;
 delete notmuch->xapian_db;
+notmuch->xapian_db = NULL;
 delete notmuch->value_range_processor;
+notmuch->value_range_processor = NULL;
+}
+
+void
+notmuch_database_destroy (notmuch_database_t *notmuch)
+{
+notmuch_database_close (notmuch);
 talloc_free (notmuch);
 }

diff --git a/lib/notmuch.h b/lib/notmuch.h
index babd208..2fb4e70 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
  *
  * After a successful call to notmuch_database_create, the returned
  * database will be open so the caller should call
- * notmuch_database_close when finished with it.
+ * notmuch_database_destroy when finished with it.
  *
  * The database will not yet have any data in it
  * (notmuch_database_create itself is a very cheap function). Messages
@@ -165,7 +165,7 @@ typedef enum {
  * An existing notmuch database can be identified by the presence of a
  * directory named ".notmuch" below 'path'.
  *
- * The caller should call notmuch_database_close when finished with
+ * The caller should call notmuch_database_destroy when finished with
  * this database.
  *
  * In case of any failure, this function returns NULL, (after printing
@@ -175,11 +175,18 @@ notmuch_database_t *
 notmuch_database_open (const char *path,
   notmuch_database_mode_t mode);

-/* Close the given notmuch database, freeing all associated
- * resources. See notmuch_database_open. */
+/* Close the given notmuch database.
+ *
+ * This function is called by notmuch_database_destroy and can be
+ * called multiple times. */
 void
 notmuch_database_close (notmuch_database_t *database);

+/* Destroy the notmuch database freeing all associated
+ * resources */
+void
+notmuch_database_destroy (notmuch_database_t *database);
+
 /* Return the database path of the given database.
  *
  * The return value is a string owned by notmuch so should not be
-- 
1.7.9.1

[PATCH 1/7] Split notmuch_database_close into two functions

[Justus Winter]

Formerly notmuch_database_close closed the xapian database and
destroyed the talloc structure associated with the notmuch database
object. Split notmuch_database_close into notmuch_database_close and
notmuch_database_destroy.

This makes it possible for long running programs to close the xapian
database and thus release the lock associated with it without
destroying the data structures obtained from it.

This also makes the api more consistent since every other data
structure has a destructor function.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 lib/database.cc |   14 --
 lib/notmuch.h   |   15 +++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 16c4354..2fefcad 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
read-write mode.\n,
 notmuch_path, version, NOTMUCH_DATABASE_VERSION);
notmuch-mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
goto DONE;
}
@@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
 } catch (const Xapian::Error error) {
fprintf (stderr, A Xapian exception occurred opening database: %s\n,
 error.get_msg().c_str());
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
 }
 
@@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
 }
 
 delete notmuch-term_gen;
+notmuch-term_gen = NULL;
 delete notmuch-query_parser;
+notmuch-query_parser = NULL;
 delete notmuch-xapian_db;
+notmuch-xapian_db = NULL;
 delete notmuch-value_range_processor;
+notmuch-value_range_processor = NULL;
+}
+
+void
+notmuch_database_destroy (notmuch_database_t *notmuch)
+{
+notmuch_database_close (notmuch);
 talloc_free (notmuch);
 }
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index babd208..2fb4e70 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
  *
  * After a successful call to notmuch_database_create, the returned
  * database will be open so the caller should call
- * notmuch_database_close when finished with it.
+ * notmuch_database_destroy when finished with it.
  *
  * The database will not yet have any data in it
  * (notmuch_database_create itself is a very cheap function). Messages
@@ -165,7 +165,7 @@ typedef enum {
  * An existing notmuch database can be identified by the presence of a
  * directory named .notmuch below 'path'.
  *
- * The caller should call notmuch_database_close when finished with
+ * The caller should call notmuch_database_destroy when finished with
  * this database.
  *
  * In case of any failure, this function returns NULL, (after printing
@@ -175,11 +175,18 @@ notmuch_database_t *
 notmuch_database_open (const char *path,
   notmuch_database_mode_t mode);
 
-/* Close the given notmuch database, freeing all associated
- * resources. See notmuch_database_open. */
+/* Close the given notmuch database.
+ *
+ * This function is called by notmuch_database_destroy and can be
+ * called multiple times. */
 void
 notmuch_database_close (notmuch_database_t *database);
 
+/* Destroy the notmuch database freeing all associated
+ * resources */
+void
+notmuch_database_destroy (notmuch_database_t *database);
+
 /* Return the database path of the given database.
  *
  * The return value is a string owned by notmuch so should not be
-- 
1.7.9.1

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

[PATCH 1/7] Split notmuch_database_close into two functions

[Justus Winter]

Formerly notmuch_database_close closed the xapian database and
destroyed the talloc structure associated with the notmuch database
object. Split notmuch_database_close into notmuch_database_close and
notmuch_database_destroy.

This makes it possible for long running programs to close the xapian
database and thus release the lock associated with it without
destroying the data structures obtained from it.

This also makes the api more consistent since every other data
structure has a destructor function.

Signed-off-by: Justus Winter <4winter at informatik.uni-hamburg.de>
---
 lib/database.cc |   14 --
 lib/notmuch.h   |   15 +++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 16c4354..2fefcad 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
 "   read-write mode.\n",
 notmuch_path, version, NOTMUCH_DATABASE_VERSION);
notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
goto DONE;
}
@@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
 } catch (const Xapian::Error ) {
fprintf (stderr, "A Xapian exception occurred opening database: %s\n",
 error.get_msg().c_str());
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
 }

@@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
 }

 delete notmuch->term_gen;
+notmuch->term_gen = NULL;
 delete notmuch->query_parser;
+notmuch->query_parser = NULL;
 delete notmuch->xapian_db;
+notmuch->xapian_db = NULL;
 delete notmuch->value_range_processor;
+notmuch->value_range_processor = NULL;
+}
+
+void
+notmuch_database_destroy (notmuch_database_t *notmuch)
+{
+notmuch_database_close (notmuch);
 talloc_free (notmuch);
 }

diff --git a/lib/notmuch.h b/lib/notmuch.h
index babd208..6114f36 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
  *
  * After a successful call to notmuch_database_create, the returned
  * database will be open so the caller should call
- * notmuch_database_close when finished with it.
+ * notmuch_database_destroy when finished with it.
  *
  * The database will not yet have any data in it
  * (notmuch_database_create itself is a very cheap function). Messages
@@ -165,7 +165,7 @@ typedef enum {
  * An existing notmuch database can be identified by the presence of a
  * directory named ".notmuch" below 'path'.
  *
- * The caller should call notmuch_database_close when finished with
+ * The caller should call notmuch_database_destroy when finished with
  * this database.
  *
  * In case of any failure, this function returns NULL, (after printing
@@ -175,11 +175,18 @@ notmuch_database_t *
 notmuch_database_open (const char *path,
   notmuch_database_mode_t mode);

-/* Close the given notmuch database, freeing all associated
- * resources. See notmuch_database_open. */
+/* Close the given notmuch database.
+ *
+ * This function is called by notmuch_database_destroyed and can be
+ * called multiple times. */
 void
 notmuch_database_close (notmuch_database_t *database);

+/* Destroy the notmuch database freeing all associated
+ * resources */
+void
+notmuch_database_destroy (notmuch_database_t *database);
+
 /* Return the database path of the given database.
  *
  * The return value is a string owned by notmuch so should not be
-- 
1.7.9.1

[PATCH 1/7] Split notmuch_database_close into two functions

[Justus Winter]

Formerly notmuch_database_close closed the xapian database and
destroyed the talloc structure associated with the notmuch database
object. Split notmuch_database_close into notmuch_database_close and
notmuch_database_destroy.

This makes it possible for long running programs to close the xapian
database and thus release the lock associated with it without
destroying the data structures obtained from it.

This also makes the api more consistent since every other data
structure has a destructor function.

Signed-off-by: Justus Winter 4win...@informatik.uni-hamburg.de
---
 lib/database.cc |   14 --
 lib/notmuch.h   |   15 +++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 16c4354..2fefcad 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -642,7 +642,7 @@ notmuch_database_open (const char *path,
read-write mode.\n,
 notmuch_path, version, NOTMUCH_DATABASE_VERSION);
notmuch-mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
goto DONE;
}
@@ -702,7 +702,7 @@ notmuch_database_open (const char *path,
 } catch (const Xapian::Error error) {
fprintf (stderr, A Xapian exception occurred opening database: %s\n,
 error.get_msg().c_str());
-   notmuch_database_close (notmuch);
+   notmuch_database_destroy (notmuch);
notmuch = NULL;
 }
 
@@ -738,9 +738,19 @@ notmuch_database_close (notmuch_database_t *notmuch)
 }
 
 delete notmuch-term_gen;
+notmuch-term_gen = NULL;
 delete notmuch-query_parser;
+notmuch-query_parser = NULL;
 delete notmuch-xapian_db;
+notmuch-xapian_db = NULL;
 delete notmuch-value_range_processor;
+notmuch-value_range_processor = NULL;
+}
+
+void
+notmuch_database_destroy (notmuch_database_t *notmuch)
+{
+notmuch_database_close (notmuch);
 talloc_free (notmuch);
 }
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index babd208..6114f36 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -133,7 +133,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
  *
  * After a successful call to notmuch_database_create, the returned
  * database will be open so the caller should call
- * notmuch_database_close when finished with it.
+ * notmuch_database_destroy when finished with it.
  *
  * The database will not yet have any data in it
  * (notmuch_database_create itself is a very cheap function). Messages
@@ -165,7 +165,7 @@ typedef enum {
  * An existing notmuch database can be identified by the presence of a
  * directory named .notmuch below 'path'.
  *
- * The caller should call notmuch_database_close when finished with
+ * The caller should call notmuch_database_destroy when finished with
  * this database.
  *
  * In case of any failure, this function returns NULL, (after printing
@@ -175,11 +175,18 @@ notmuch_database_t *
 notmuch_database_open (const char *path,
   notmuch_database_mode_t mode);
 
-/* Close the given notmuch database, freeing all associated
- * resources. See notmuch_database_open. */
+/* Close the given notmuch database.
+ *
+ * This function is called by notmuch_database_destroyed and can be
+ * called multiple times. */
 void
 notmuch_database_close (notmuch_database_t *database);
 
+/* Destroy the notmuch database freeing all associated
+ * resources */
+void
+notmuch_database_destroy (notmuch_database_t *database);
+
 /* Return the database path of the given database.
  *
  * The return value is a string owned by notmuch so should not be
-- 
1.7.9.1

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