Re: [PATCH] lib: make find_message{,by_filename) report errors

[Sebastian Spaeth]

The new API looks sane and much better to me.

+1, just give me plenty of time to catch up before releasing once this
goes in :-)

Sebastian


pgpnZQGJSMlVt.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] lib: make find_message{,by_filename) report errors

[Sebastian Spaeth]

The new API looks sane and much better to me.

+1, just give me plenty of time to catch up before releasing once this
goes in :-)

Sebastian
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 

[PATCH] lib: make find_message{,by_filename) report errors

[Ali Polatel]

David Bremner yazm??:
>On Sat,  1 Oct 2011 11:12:23 +0300, Ali Polatel  wrote:
>> From: Ali Polatel 
>>
>> Looks like the patch did not make it correctly the first time.
>> Resending using git-send-email?
>>
>> You may also find the commit in my notmuch repository:
>> git://github.com/alip/notmuch.git branch: find_message
>
>
>Hi Ali;
>
>Thanks for reworking this patch. I looked at branch find_message-v2
>in your repo. I have a few comments.

Thanks for going over the patch, expect a new set of patches soon!

>- In the comments for _resolve_message_id_to_thread_id I guess thread_id
>  should be thread_id_ret?

Fixed.

>- in notmuch_database_find_message_by_file_name, I'm not sure why you
>  set status to NOTMUCH_STATUS_OUT_OF_MEMORY in the catch block. Is this
>  a typo?

Looks like a copy & paste error. I must have blindly copied the error
from the previous block. Fixed.

>- after the DONE: label of the same routine, how is *message_ret destroyed?
>  does it need to wait until the talloc context "notmuch" is freed?

Yes, I have modified it to call notmuch_message_destroy() in case
'*message_ret' is non-NULL after the DONE:

>- I don't really get the change of user to caller around notmuch.h:286
>  It is not a big deal, but I guess we should try to be consistent.

I don't get what you mean by consistency here but this hunk is unrelated
to the problem which the patch is trying to address.
Reverted.

>David
>
>

 -alip
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: 

Re: [PATCH] lib: make find_message{,by_filename) report errors

[David Bremner]

On Sat,  1 Oct 2011 11:12:23 +0300, Ali Polatel pola...@gmail.com wrote:
 From: Ali Polatel a...@exherbo.org
 
 Looks like the patch did not make it correctly the first time.
 Resending using git-send-email™
 
 You may also find the commit in my notmuch repository:
 git://github.com/alip/notmuch.git branch: find_message


Hi Ali;

Thanks for reworking this patch. I looked at branch find_message-v2
in your repo. I have a few comments.

- In the comments for _resolve_message_id_to_thread_id I guess thread_id
  should be thread_id_ret?

- in notmuch_database_find_message_by_file_name, I'm not sure why you
  set status to NOTMUCH_STATUS_OUT_OF_MEMORY in the catch block. Is this
  a typo?

- after the DONE: label of the same routine, how is *message_ret destroyed?
  does it need to wait until the talloc context notmuch is freed?

- I don't really get the change of user to caller around notmuch.h:286
  It is not a big deal, but I guess we should try to be consistent.

David

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

Re: [PATCH] lib: make find_message{,by_filename) report errors

[Ali Polatel]

David Bremner yazmış:

On Sat,  1 Oct 2011 11:12:23 +0300, Ali Polatel pola...@gmail.com wrote:

From: Ali Polatel a...@exherbo.org

Looks like the patch did not make it correctly the first time.
Resending using git-send-email™

You may also find the commit in my notmuch repository:
git://github.com/alip/notmuch.git branch: find_message



Hi Ali;

Thanks for reworking this patch. I looked at branch find_message-v2
in your repo. I have a few comments.


Thanks for going over the patch, expect a new set of patches soon!


- In the comments for _resolve_message_id_to_thread_id I guess thread_id
 should be thread_id_ret?


Fixed.


- in notmuch_database_find_message_by_file_name, I'm not sure why you
 set status to NOTMUCH_STATUS_OUT_OF_MEMORY in the catch block. Is this
 a typo?


Looks like a copy  paste error. I must have blindly copied the error
from the previous block. Fixed.


- after the DONE: label of the same routine, how is *message_ret destroyed?
 does it need to wait until the talloc context notmuch is freed?


Yes, I have modified it to call notmuch_message_destroy() in case
'*message_ret' is non-NULL after the DONE:


- I don't really get the change of user to caller around notmuch.h:286
 It is not a big deal, but I guess we should try to be consistent.


I don't get what you mean by consistency here but this hunk is unrelated
to the problem which the patch is trying to address.
Reverted.


David




-alip


pgpwbwIKesktQ.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] lib: make find_message{,by_filename) report errors

[David Bremner]

On Sat,  1 Oct 2011 11:12:23 +0300, Ali Polatel  wrote:
> From: Ali Polatel 
> 
> Looks like the patch did not make it correctly the first time.
> Resending using git-send-email?
> 
> You may also find the commit in my notmuch repository:
> git://github.com/alip/notmuch.git branch: find_message


Hi Ali;

Thanks for reworking this patch. I looked at branch find_message-v2
in your repo. I have a few comments.

- In the comments for _resolve_message_id_to_thread_id I guess thread_id
  should be thread_id_ret?

- in notmuch_database_find_message_by_file_name, I'm not sure why you
  set status to NOTMUCH_STATUS_OUT_OF_MEMORY in the catch block. Is this
  a typo?

- after the DONE: label of the same routine, how is *message_ret destroyed?
  does it need to wait until the talloc context "notmuch" is freed?

- I don't really get the change of user to caller around notmuch.h:286
  It is not a big deal, but I guess we should try to be consistent.

David

[PATCH] lib: make find_message{,by_filename) report errors

[Ali Polatel]

From: Ali Polatel 

Looks like the patch did not make it correctly the first time.
Resending using git-send-email?

You may also find the commit in my notmuch repository:
git://github.com/alip/notmuch.git branch: find_message

Ali Polatel (1):
  lib: make find_message{,by_filename) report errors

 lib/database.cc   |   89 ++--
 lib/message.cc|6 ++--
 lib/notmuch.h |   61 +---
 notmuch-new.c |4 ++-
 notmuch-restore.c |   11 --
 5 files changed, 114 insertions(+), 57 deletions(-)

-- 
1.7.6.1

[PATCH] lib: make find_message{,by_filename) report errors

[Ali Polatel]

From: Ali Polatel a...@exherbo.org

Looks like the patch did not make it correctly the first time.
Resending using git-send-email™

You may also find the commit in my notmuch repository:
git://github.com/alip/notmuch.git branch: find_message

Ali Polatel (1):
  lib: make find_message{,by_filename) report errors

 lib/database.cc   |   89 ++--
 lib/message.cc|6 ++--
 lib/notmuch.h |   61 +---
 notmuch-new.c |4 ++-
 notmuch-restore.c |   11 --
 5 files changed, 114 insertions(+), 57 deletions(-)

-- 
1.7.6.1

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

[PATCH] lib: make find_message{,by_filename) report errors

[Ali Polatel]

Previously, the functions notmuch_database_find_message() and
notmuch_database_find_message_by_filename() functions did not properly
report error condition to the library user.

For more information, read the thread on the notmuch mailing list
starting with my mail id:871uv2unfd@gmail.com

Make these functions accept a pointer to 'notmuch_message_t' as argument
and return notmuch_status_t which may be used to check for any error
condition.

restore: Modify for the new notmuch_database_find_message()
new: Modify for the new notmuch_database_find_message_by_filename()
---
 lib/database.cc   |   89 ++--
 lib/message.cc|6 ++--
 lib/notmuch.h |   61 +---
 notmuch-new.c |4 ++-
 notmuch-restore.c |   11 --
 5 files changed, 114 insertions(+), 57 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 9299c8d..6641aa5 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -360,13 +360,17 @@ _message_id_compressed (void *ctx, const char *message_id)
 return compressed;
 }
 
-notmuch_message_t *
+notmuch_status_t
 notmuch_database_find_message (notmuch_database_t *notmuch,
-  const char *message_id)
+  const char *message_id,
+  notmuch_message_t **message)
 {
 notmuch_private_status_t status;
 unsigned int doc_id;
 
+if (message == NULL)
+   return NOTMUCH_STATUS_NULL_POINTER;
+
 if (strlen (message_id)  NOTMUCH_MESSAGE_ID_MAX)
message_id = _message_id_compressed (notmuch, message_id);
 
@@ -375,14 +379,20 @@ notmuch_database_find_message (notmuch_database_t 
*notmuch,
   message_id, doc_id);
 
if (status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND)
-   return NULL;
+   *message = NULL;
+   else {
+   *message = _notmuch_message_create (notmuch, notmuch, doc_id, NULL);
+   if (*message == NULL)
+   return NOTMUCH_STATUS_OUT_OF_MEMORY;
+   }
 
-   return _notmuch_message_create (notmuch, notmuch, doc_id, NULL);
+   return NOTMUCH_STATUS_SUCCESS;
 } catch (const Xapian::Error error) {
fprintf (stderr, A Xapian exception occurred finding message: %s.\n,
 error.get_msg().c_str());
notmuch-exception_reported = TRUE;
-   return NULL;
+   *message = NULL;
+   return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
 }
 }
 
@@ -1311,7 +1321,8 @@ _get_metadata_thread_id_key (void *ctx, const char 
*message_id)
 
 /* Find the thread ID to which the message with 'message_id' belongs.
  *
- * Always returns a newly talloced string belonging to 'ctx'.
+ * Note: 'thread_id' must not be NULL!
+ * On success '*thread_id' is set to a newly talloced string belonging to 
'ctx'.
  *
  * Note: If there is no message in the database with the given
  * 'message_id' then a new thread_id will be allocated for this
@@ -1319,25 +1330,29 @@ _get_metadata_thread_id_key (void *ctx, const char 
*message_id)
  * thread ID can be looked up if the message is added to the database
  * later).
  */
-static const char *
+static notmuch_status_t
 _resolve_message_id_to_thread_id (notmuch_database_t *notmuch,
  void *ctx,
- const char *message_id)
+ const char *message_id,
+ const char **thread_id)
 {
+notmuch_status_t status;
 notmuch_message_t *message;
 string thread_id_string;
-const char *thread_id;
 char *metadata_key;
 Xapian::WritableDatabase *db;
 
-message = notmuch_database_find_message (notmuch, message_id);
+status = notmuch_database_find_message (notmuch, message_id, message);
+
+if (status)
+   return status;
 
 if (message) {
-   thread_id = talloc_steal (ctx, notmuch_message_get_thread_id (message));
+   *thread_id = talloc_steal (ctx, notmuch_message_get_thread_id 
(message));
 
notmuch_message_destroy (message);
 
-   return thread_id;
+   return NOTMUCH_STATUS_SUCCESS;
 }
 
 /* Message has not been seen yet.
@@ -1351,15 +1366,15 @@ _resolve_message_id_to_thread_id (notmuch_database_t 
*notmuch,
 thread_id_string = notmuch-xapian_db-get_metadata (metadata_key);
 
 if (thread_id_string.empty()) {
-   thread_id = _notmuch_database_generate_thread_id (notmuch);
-   db-set_metadata (metadata_key, thread_id);
+   *thread_id = talloc_strdup(ctx, _notmuch_database_generate_thread_id 
(notmuch));
+   db-set_metadata (metadata_key, *thread_id);
 } else {
-   thread_id = thread_id_string.c_str();
+   *thread_id = talloc_strdup(ctx, thread_id_string.c_str());
 }
 
 talloc_free (metadata_key);
 
-return talloc_strdup (ctx, thread_id);
+return NOTMUCH_STATUS_SUCCESS;
 }
 
 static notmuch_status_t
@@