[PATCH] lib/database: reduce try block scope to things that really need it

classic Classic list List threaded Threaded
3 messages Options
Jani Nikula Jani Nikula
Reply | Threaded
Open this post in threaded view
|

[PATCH] lib/database: reduce try block scope to things that really need it

No need to maintain the pure C stuff within a try block, it's arguably
confusing. This also reduces indent for a bunch of code. No functional
changes.
---
 lib/database.cc | 80 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 2d19f20c4b5e..aac8d62fd27d 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -2450,53 +2450,53 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
     if (ret)
  goto DONE;
 
-    try {
- /* Before we do any real work, (especially before doing a
- * potential SHA-1 computation on the entire file's contents),
- * let's make sure that what we're looking at looks like an
- * actual email message.
- */
- from = _notmuch_message_file_get_header (message_file, "from");
- subject = _notmuch_message_file_get_header (message_file, "subject");
- to = _notmuch_message_file_get_header (message_file, "to");
+    /* Before we do any real work, (especially before doing a
+     * potential SHA-1 computation on the entire file's contents),
+     * let's make sure that what we're looking at looks like an
+     * actual email message.
+     */
+    from = _notmuch_message_file_get_header (message_file, "from");
+    subject = _notmuch_message_file_get_header (message_file, "subject");
+    to = _notmuch_message_file_get_header (message_file, "to");
+
+    if ((from == NULL || *from == '\0') &&
+ (subject == NULL || *subject == '\0') &&
+ (to == NULL || *to == '\0')) {
+ ret = NOTMUCH_STATUS_FILE_NOT_EMAIL;
+ goto DONE;
+    }
 
- if ((from == NULL || *from == '\0') &&
-    (subject == NULL || *subject == '\0') &&
-    (to == NULL || *to == '\0'))
- {
-    ret = NOTMUCH_STATUS_FILE_NOT_EMAIL;
-    goto DONE;
- }
+    /* Now that we're sure it's mail, the first order of business
+     * is to find a message ID (or else create one ourselves).
+     */
+    header = _notmuch_message_file_get_header (message_file, "message-id");
+    if (header && *header != '\0') {
+ message_id = _parse_message_id (message_file, header, NULL);
 
- /* Now that we're sure it's mail, the first order of business
- * is to find a message ID (or else create one ourselves). */
+ /* So the header value isn't RFC-compliant, but it's
+ * better than no message-id at all.
+ */
+ if (message_id == NULL)
+    message_id = talloc_strdup (message_file, header);
+    }
 
- header = _notmuch_message_file_get_header (message_file, "message-id");
- if (header && *header != '\0') {
-    message_id = _parse_message_id (message_file, header, NULL);
+    if (message_id == NULL ) {
+ /* No message-id at all, let's generate one by taking a
+ * hash over the file's contents.
+ */
+ char *sha1 = _notmuch_sha1_of_file (filename);
 
-    /* So the header value isn't RFC-compliant, but it's
-     * better than no message-id at all. */
-    if (message_id == NULL)
- message_id = talloc_strdup (message_file, header);
+ /* If that failed too, something is really wrong. Give up. */
+ if (sha1 == NULL) {
+    ret = NOTMUCH_STATUS_FILE_ERROR;
+    goto DONE;
  }
 
- if (message_id == NULL ) {
-    /* No message-id at all, let's generate one by taking a
-     * hash over the file's contents. */
-    char *sha1 = _notmuch_sha1_of_file (filename);
-
-    /* If that failed too, something is really wrong. Give up. */
-    if (sha1 == NULL) {
- ret = NOTMUCH_STATUS_FILE_ERROR;
- goto DONE;
-    }
-
-    message_id = talloc_asprintf (message_file,
-  "notmuch-sha1-%s", sha1);
-    free (sha1);
- }
+ message_id = talloc_asprintf (message_file, "notmuch-sha1-%s", sha1);
+ free (sha1);
+    }
 
+    try {
  /* Now that we have a message ID, we get a message object,
  * (which may or may not reference an existing document in the
  * database). */
--
2.10.2

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] lib/database: reduce try block scope to things that really need it

Jani Nikula <[hidden email]> writes:

> No need to maintain the pure C stuff within a try block, it's arguably
> confusing. This also reduces indent for a bunch of code. No functional
> changes.

I didn't trace all the code paths, but there is the possibility this
will allow some uncaught exceptions deeper in the "pure C stuff" to
escape (along the lines of what we saw with
_notmuch_message_ensure_metadata. As long as we agree that those are
bugs that should be fixed, the beginning of a release cycle seems like a
reasonable time to deal with that.

d
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] lib/database: reduce try block scope to things that really need it

In reply to this post by Jani Nikula
Jani Nikula <[hidden email]> writes:

> No need to maintain the pure C stuff within a try block, it's arguably
> confusing. This also reduces indent for a bunch of code. No functional
> changes.

pushed to master,

d
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch