Re: [notmuch] [PATCH] notmuch-count: make sure all created items are freed, even in error paths
On Tue, 24 Nov 2009 16:22:02 +0200, Dirk-Jan C. Binnema wrote: > > Another minor patch, fixing a couple of resource leaks in error paths. Thanks, Dirk-Jan, but we don't actually need this kind of care in some cases, because these aren't leaks in a permanent sense since they are talloc-based. Instead these "leaks" simply mean that on the error paths these objects will live slightly longer than normal, until the caller frees the talloc context. And I'm perfectly happy to have slightly longer-lived objects in the error paths, (while still guaranteeing leak-free behavior in all cases). And I definitely like the easier-to-read-and-verify code that comes with the early return values. Now, we do have to be careful if there are side effects from having an object live longer, (such as holding a database write lock or something). So if there's a fix for something like that in an error path, then yes, we'll definitely want that. -Carl ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[notmuch] [PATCH] notmuch-count: make sure all created items are freed, even in error paths
On Tue, 24 Nov 2009 16:22:02 +0200, Dirk-Jan C. Binnema wrote: > > Another minor patch, fixing a couple of resource leaks in error paths. Thanks, Dirk-Jan, but we don't actually need this kind of care in some cases, because these aren't leaks in a permanent sense since they are talloc-based. Instead these "leaks" simply mean that on the error paths these objects will live slightly longer than normal, until the caller frees the talloc context. And I'm perfectly happy to have slightly longer-lived objects in the error paths, (while still guaranteeing leak-free behavior in all cases). And I definitely like the easier-to-read-and-verify code that comes with the early return values. Now, we do have to be careful if there are side effects from having an object live longer, (such as holding a database write lock or something). So if there's a fix for something like that in an error path, then yes, we'll definitely want that. -Carl
[notmuch] [PATCH] notmuch-count: make sure all created items are freed, even in error paths
Another minor patch, fixing a couple of resource leaks in error paths.
---
notmuch-count.c | 52
1 files changed, 36 insertions(+), 16 deletions(-)
diff --git a/notmuch-count.c b/notmuch-count.c
index 77aa433..b5808f5 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -28,7 +28,8 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
notmuch_database_t *notmuch;
notmuch_query_t *query;
char *query_str;
-int i;
+int i, retval;
+
#if 0
char *opt, *end;
int i, first = 0, max_threads = -1;
@@ -76,35 +77,54 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
argc -= i;
argv += i;
+config= NULL;
+query_str = NULL;
+notmuch = NULL;
+query = NULL;
+
config = notmuch_config_open (ctx, NULL, NULL);
-if (config == NULL)
- return 1;
-
-notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
-NOTMUCH_DATABASE_MODE_READ_ONLY);
-if (notmuch == NULL)
- return 1;
-
+if (config == NULL) {
+ retval = 1;
+ goto out;
+}
+
query_str = query_string_from_args (ctx, argc, argv);
if (query_str == NULL) {
fprintf (stderr, "Out of memory.\n");
- return 1;
+ retval = 1;
+ goto out;
}
if (*query_str == '\0') {
fprintf (stderr, "Error: notmuch count requires at least one count
term.\n");
- return 1;
+ retval = 1;
+ goto out;
}
-
+
+notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
+NOTMUCH_DATABASE_MODE_READ_ONLY);
+if (notmuch == NULL) {
+ fprintf (stderr, "Failed to open database at %s\n",
+notmuch_config_get_database_path (config));
+ retval = 1;
+ goto out;
+}
+
query = notmuch_query_create (notmuch, query_str);
if (query == NULL) {
fprintf (stderr, "Out of memory\n");
- return 1;
+ retval = 1;
+ goto out;
}
printf ("%u\n", notmuch_query_count_messages(query));
+retval = 0;
+out:
+talloc_free (query_str);
+notmuch_config_close (config);
notmuch_query_destroy (query);
-notmuch_database_close (notmuch);
-
-return 0;
+if (notmuch) /* _close does not allow NULL */
+ notmuch_database_close (notmuch);
+
+return retval;
}
--
1.6.3.3
