[PATCH 1/3] lib: Don't delete uninitialized pointers

[PATCH 1/3] lib: Don't delete uninitialized pointers

[Austin Clements]

In the error-handling paths of notmuch_database_open, we call
notmuch_database_close, which "delete"s several objects referenced by
the notmuch_database_t object.  However, some of these pointers may be
uninitialized, resulting in undefined behavior.  Hence, allocate the
notmuch_database_t with talloc_zero to make sure these pointers are
NULL so that "delete"ing them is harmless.
---
 lib/database.cc |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 8103bd9..a6d15a1 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -617,7 +617,7 @@ notmuch_database_open (const char *path,
 	initialized = 1;
     }
 
-    notmuch = talloc (NULL, notmuch_database_t);
+    notmuch = talloc_zero (NULL, notmuch_database_t);
     notmuch->exception_reported = FALSE;
     notmuch->path = talloc_strdup (notmuch, path);
 
-- 
1.7.7.3

[PATCH 2/3] lib: Release resources if notmuch_database_open fails

[Austin Clements]

Previously, if a Xapian exception occurred in notmuch_database_open,
we failed to clean up the allocated notmuch_database_t object.
---
 lib/database.cc |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index a6d15a1..94022d7 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -703,6 +703,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 = NULL;
     }
 
-- 
1.7.7.3

[PATCH 3/3] lib: Use talloc to simplify cleanup in notmuch_database_open

[Austin Clements]

Previously, we manually "free"d various pointers in
notmuch_database_open.  Use a local talloc context instead to simplify
cleanup and eliminate various NULL pointer initializations and
conditionals.
---
 lib/database.cc |   14 +++++---------
 1 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 94022d7..c928d02 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -582,15 +582,15 @@ notmuch_database_t *
 notmuch_database_open (const char *path,
 		       notmuch_database_mode_t mode)
 {
+    void *local = talloc_new (NULL);
     notmuch_database_t *notmuch = NULL;
-    char *notmuch_path = NULL, *xapian_path = NULL;
+    char *notmuch_path, *xapian_path;
     struct stat st;
     int err;
     unsigned int i, version;
     static int initialized = 0;
 
-    if (asprintf (&notmuch_path, "%s/%s", path, ".notmuch") == -1) {
-	notmuch_path = NULL;
+    if (! (notmuch_path = talloc_asprintf (local, "%s/%s", path, ".notmuch"))) {
 	fprintf (stderr, "Out of memory\n");
 	goto DONE;
     }
@@ -602,8 +602,7 @@ notmuch_database_open (const char *path,
 	goto DONE;
     }
 
-    if (asprintf (&xapian_path, "%s/%s", notmuch_path, "xapian") == -1) {
-	xapian_path = NULL;
+    if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) {
 	fprintf (stderr, "Out of memory\n");
 	goto DONE;
     }
@@ -708,10 +707,7 @@ notmuch_database_open (const char *path,
     }
 
   DONE:
-    if (notmuch_path)
-	free (notmuch_path);
-    if (xapian_path)
-	free (xapian_path);
+    talloc_free (local);
 
     return notmuch;
 }
-- 
1.7.7.3

Re: [PATCH 3/3] lib: Use talloc to simplify cleanup in notmuch_database_open

[Tomi Ollila]

LGTM. all 3.

Tomi

Re: [PATCH 1/3] lib: Don't delete uninitialized pointers

[Dmitry Kurochkin]

Hi Austin.

All 3 patches look good to me.

Regards,
  Dmitry

Re: [PATCH 3/3] lib: Use talloc to simplify cleanup in notmuch_database_open

[David Bremner]

On Sun, 29 Jan 2012 00:50:10 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Previously, we manually "free"d various pointers in
> notmuch_database_open.  Use a local talloc context instead to simplify
> cleanup and eliminate various NULL pointer initializations and
> conditionals.

pushed all 3.

d