Re: [PATCH 4/6] cli: intialize crypto structure in show and reply
On Fri, May 18 2012, Daniel Kahn Gillmor wrote:
>
> The real tradeoff in this choice is whether we prefer:
>
> a) more compact code to facilitate quick reading by experts
>
>or
>
> b) more verbose code to facilitate comprehension by the non-expert.
>
> I started this discussion leaning strongly toward the (b) perspective.
> But now that i know the relevant bits of the standard, i can sympathize
> with the (a) perspective as well :P
>
> Overall, i think i'm still in the (b) camp. But i think it's more
> important that we don't allow dithering over this issue to prevent the
> inclusion of this patch series, which is a step in the right direction
> for handling S/MIME messages as well as PGP/MIME.
I also think it is good to see explicit initializations when those aren't
needed but clarifies the code. After all it doesn't generate any extra
code to the target module. Also the &(params->crypto) is good from
clarification point of view.
Austin's .crypto { ... } initialization looks good & clear; In case there
will be new version of this patch series I'd like to see that used...
> --dkg
>
> PS gcc's -pedantic argument provides the following warning:
>
> error: ISO C90 forbids specifying subobject to initialize
>
> So we probably want to specify -std=c99 at least to ensure our choice of
> subobject initialization is respected.
In order to do that id:"cover.1325977940.git.j...@nikula.org" needs to
be applied (and probably rebased).
> [0] http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf
Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/6] cli: intialize crypto structure in show and reply
On Thu, 17 May 2012, Jameson Graef Rollins wrote: > On Thu, May 17 2012, Jani Nikula wrote: >> The values are not undefined, they are properly initialized, and we can >> count on it. For sure, not maybe. If you want to explicitly set them for >> clarity, it's a matter of taste. Personally I find it too verbose, but then >> again notmuch code is generally fairly verbose. > > I want them explicitly set for clarity, as well as safety. Code is > meant to be read by humans, not computers. Brevity is not always a > virtue if it sacrifices clarity. It's much nicer to have the defaults > clearly stated in the initialization, than to force the reader to > understand how the initialization works and to interpret what that means > for the current case. I also don't think it's safe to assume that the > variables will be always be "properly" initialized in your favor in > perpetuity. It's much safer to explicitly set them to what you want > them to be rather than just assume they'll be set correctly. In short, when you read enough code, having everything explicitly stated becomes a burden. It's explicit, but you have to read it all, even when there really is no need to. >> If you insist on it, please at least drop the extra temp crypto >> variable, and initialize the struct in one initializer. > > I don't see why this matters either. Again, I think this is just a > matter of taste. I would rather the code be verbose where clarity > requires it, rather than always trying to make the code as terse as > possible. You introduce an extra variable that every reader of your code has to track down to realize that it's only ever used once to initialize another variable. You make code harder for other people to read. I have now offered my review and opinions on the matter; I will not pursue this discussion further. BR, Jani.
[alot] announcing v0.3.1
Quoting Jameson Graef Rollins (2012-05-17 16:42:53) > On Thu, May 17 2012, Patrick Totzke wrote: > > With David Bremners help I have started hacking together some metadata to > > build > > a debian package. This can be found in branch 'debian' but I presume there > > are still > > some things to be done on that (manpage generation..). > > Hi, Patrick. I put together some Debian packaging a while back that I > never did anything with: > > git://finestructure.net/alot [debian] > > It's a bit old, but it's mostly complete. Hopefully you'll find it > helpful. Hey Jamie, Ah yes, I forgot about this. Thanks for reminding me! IIRC, I could not get this to "run through" on my machine but thats surely due to my limited knowledge about debian packaging. I'll have a look at this again and compare it with my stuff. Cheers, /p -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: signature URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120517/91bd18a9/attachment.pgp>
[PATCH 1/3] NEWS: Insert markdown formatting commands in 0.13 section text
On Thu, May 17 2012, David Bremner wrote: > Tomi Ollila writes: >> >> id:"1337152910-22371-4-git-send-email-tomi.ollila at iki.fi" >> id:"1337152910-22371-5-git-send-email-tomi.ollila at iki.fi" >> id:"1337152910-22371-6-git-send-email-tomi.ollila at iki.fi" >> id:"1337152910-22371-7-git-send-email-tomi.ollila at iki.fi" > > I pushed those to release. I'll merge them to master after a bit; > hopefully we'll figure out that reply encoding thing and do a bug fix > release. I got it working by doing this: --8<8<8<8<8<8<8<8<8<8<8<8<-- diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index 7fa441a..7523bb6 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -244,7 +244,9 @@ the given type." current buffer, if possible." (let ((display-buffer (current-buffer))) (with-temp-buffer - (let* ((charset (plist-get part :content-charset)) + ;; (let* ((charset (or (plist-get part :content-charset) "utf-8")) + ;; (let* ((charset (or (plist-get part :content-charset) "latin-1")) + (let* ((charset (or (plist-get part :content-charset) 'gnus-decoded)) (handle (mm-make-handle (current-buffer) `(,content-type (charset . ,charset) ;; If the user wants the part inlined, insert the content and ;; test whether we are able to inline it (which includes both --8<8<8<8<8<8<8<8<8<8<8<8<-- (using "latin-1" also provided desired results... but with "utf-8" results were the same as with default 'nil) But, for consistency (with `notmuch show ...` handling), when content-type is "text/plain" (and as `notmuch reply --format=json` provides text/plain content inline) we should use the construct `(insert (notmuch-get-bodypart-content msg part nth ...)` to insert that content (as done before f6c170fabca8f39e74705e3813504137811bf162 and is also done in `notmuch-show-insert-part-text/plain`). If anyone is faster than me to provide patch what would be fine ;) > d Tomi
[PATCH 5/9] new: Remove workaround for detecting newly created directory objects
Previously, notmuch_database_get_directory did not indicate whether or
not the returned directory object was newly created, which required a
workaround to distinguish newly created directory objects with no
child messages from directory objects that had no mtime set but did
have child messages. Now that notmuch_database_get_directory
distinguishes whether or not the directory object exists in the
database, this workaround is no longer necessary.
---
notmuch-new.c | 36 +++-
1 file changed, 7 insertions(+), 29 deletions(-)
diff --git a/notmuch-new.c b/notmuch-new.c
index a3a8bec..72dd558 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -256,7 +256,7 @@ add_files_recursive (notmuch_database_t *notmuch,
notmuch_filenames_t *db_subdirs = NULL;
time_t stat_time;
struct stat st;
-notmuch_bool_t is_maildir, new_directory;
+notmuch_bool_t is_maildir;
const char **tag;
if (stat (path, &st)) {
@@ -281,33 +281,12 @@ add_files_recursive (notmuch_database_t *notmuch,
}
db_mtime = directory ? notmuch_directory_get_mtime (directory) : 0;
-new_directory = db_mtime ? FALSE : TRUE;
-
-/* XXX This is a temporary workaround. If we don't update the
- * database mtime until after processing messages in this
- * directory, then a 0 mtime is *not* sufficient to indicate that
- * this directory has no messages or subdirs in the database (for
- * example, if an earlier run skipped the mtime update because
- * fs_mtime == stat_time, or was interrupted before updating the
- * mtime at the end). To address this, we record a (bogus)
- * non-zero value before processing any child messages so that a
- * later run won't mistake this for a new directory (and, for
- * example, fail to detect removed files and subdirs).
- *
- * A better solution would be for notmuch_database_get_directory
- * to indicate if it really created a new directory or not, either
- * by a new out-argument, or by recording this information and
- * providing an accessor.
- */
-if (new_directory && directory)
- notmuch_directory_set_mtime (directory, -1);
-
/* If the database knows about this directory, then we sort based
* on strcmp to match the database sorting. Otherwise, we can do
* inode-based sorting for faster filesystem operation. */
num_fs_entries = scandir (path, &fs_entries, 0,
- new_directory ?
- dirent_sort_inode : dirent_sort_strcmp_name);
+ directory ?
+ dirent_sort_strcmp_name : dirent_sort_inode);
if (num_fs_entries == -1) {
fprintf (stderr, "Error opening directory %s: %s\n",
@@ -376,13 +355,12 @@ add_files_recursive (notmuch_database_t *notmuch,
* being discovered until the clock catches up and the directory
* is modified again).
*/
-if (fs_mtime == db_mtime)
+if (directory && fs_mtime == db_mtime)
goto DONE;
-/* new_directory means a directory that the database has never
- * seen before. In that case, we can simply leave db_files and
- * db_subdirs NULL. */
-if (!new_directory) {
+/* If the database has never seen this directory before, we can
+ * simply leave db_files and db_subdirs NULL. */
+if (directory) {
db_files = notmuch_directory_get_child_files (directory);
db_subdirs = notmuch_directory_get_child_directories (directory);
}
--
1.7.10
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 8/9] python: Remove find_message_by_filename workaround
Now that notmuch_database_find_message_by_filename works on read-only
databases, remove the workaround that disabled it on read-write
databases.
This also adds a regression test for find_message_by_filename.
---
bindings/python/notmuch/database.py |9 -
test/python |8
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/bindings/python/notmuch/database.py
b/bindings/python/notmuch/database.py
index ff89818..e5c74cf 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -526,19 +526,10 @@ class Database(object):
retry.
:raises: :exc:`NotInitializedError` if the database was not
intitialized.
-:raises: :exc:`ReadOnlyDatabaseError` if the database has not been
- opened in read-write mode
*Added in notmuch 0.9*"""
self._assert_db_is_initialized()
-# work around libnotmuch calling exit(3), see
-# id:20120221002921.8534.57...@thinkbox.jade-hamburg.de
-# TODO: remove once this issue is resolved
-if self.mode != Database.MODE.READ_WRITE:
-raise ReadOnlyDatabaseError('The database has to be opened in '
-'read-write mode for get_directory')
-
msg_p = NotmuchMessageP()
status = Database._find_message_by_filename(self._db, _str(filename),
byref(msg_p))
diff --git a/test/python b/test/python
index 6018c2d..3f03a2e 100755
--- a/test/python
+++ b/test/python
@@ -28,4 +28,12 @@ EOF
notmuch search --sort=oldest-first --output=messages tag:inbox | sed s/^id://
> EXPECTED
test_expect_equal_file OUTPUT EXPECTED
+test_begin_subtest "get non-existent file"
+test_python
[PATCH 7/9] lib: Make notmuch_database_find_message_by_filename not crash on read-only databases
Previously, _notmuch_database_filename_to_direntry would abort with an
internal error when called on a read-only database. Now that creating
the directory document is optional,
notmuch_database_find_message_by_filename can disable directory
document creation (as it should) and, as a result, not abort on
read-only databases.
---
lib/database.cc |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index e27a0e1..761dc1a 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1895,8 +1895,8 @@ notmuch_database_find_message_by_filename
(notmuch_database_t *notmuch,
try {
status = _notmuch_database_filename_to_direntry (
- local, notmuch, filename, NOTMUCH_FIND_CREATE, &direntry);
- if (status)
+ local, notmuch, filename, NOTMUCH_FIND_LOOKUP, &direntry);
+ if (status || !direntry)
goto DONE;
term = talloc_asprintf (local, "%s%s", prefix, direntry);
--
1.7.10
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/9] lib: Make directory document creation optional for _notmuch_directory_create
Previously this function would create directory documents if they
didn't exist. As a result, it could only be used on writable
databases. This adds an argument to make creation optional and to
make this function work on read-only databases. We use a flag
argument to avoid a bare boolean and to permit future expansion.
Both callers have been updated, but currently retain the old behavior.
We'll take advantage of the new argument in the following patches.
---
lib/database.cc |6 +++---
lib/directory.cc | 33 -
lib/notmuch-private.h |8
3 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index f8c4a7d..df996a9 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -956,7 +956,7 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
document.get_value (NOTMUCH_VALUE_TIMESTAMP));
directory = _notmuch_directory_create (notmuch, term.c_str() +
10,
- &status);
+ NOTMUCH_FIND_CREATE,
&status);
notmuch_directory_set_mtime (directory, mtime);
notmuch_directory_destroy (directory);
}
@@ -1210,7 +1210,7 @@ _notmuch_database_find_directory_id (notmuch_database_t
*notmuch,
return NOTMUCH_STATUS_SUCCESS;
}
-directory = _notmuch_directory_create (notmuch, path, &status);
+directory = _notmuch_directory_create (notmuch, path, NOTMUCH_FIND_CREATE,
&status);
if (status) {
*directory_id = -1;
return status;
@@ -1320,7 +1320,7 @@ notmuch_database_get_directory (notmuch_database_t
*notmuch,
return NOTMUCH_STATUS_READ_ONLY_DATABASE;
try {
- *directory = _notmuch_directory_create (notmuch, path, &status);
+ *directory = _notmuch_directory_create (notmuch, path,
NOTMUCH_FIND_CREATE, &status);
} catch (const Xapian::Error &error) {
fprintf (stderr, "A Xapian exception occurred getting directory: %s.\n",
error.get_msg().c_str());
diff --git a/lib/directory.cc b/lib/directory.cc
index 70e1693..83bb19b 100644
--- a/lib/directory.cc
+++ b/lib/directory.cc
@@ -82,28 +82,41 @@ find_directory_document (notmuch_database_t *notmuch,
return NOTMUCH_PRIVATE_STATUS_SUCCESS;
}
+/* Find or create a directory document.
+ *
+ * 'path' should be a path relative to the path of 'database', or else
+ * should be an absolute path with initial components that match the
+ * path of 'database'.
+ *
+ * If (flags & NOTMUCH_FIND_CREATE), then the directory document will
+ * be created if it does not exist. Otherwise, if the directory
+ * document does not exist, *status_ret is set to
+ * NOTMUCH_STATUS_SUCCESS and this returns NULL.
+ */
notmuch_directory_t *
_notmuch_directory_create (notmuch_database_t *notmuch,
const char *path,
+ notmuch_find_flags_t flags,
notmuch_status_t *status_ret)
{
Xapian::WritableDatabase *db;
notmuch_directory_t *directory;
notmuch_private_status_t private_status;
const char *db_path;
+notmuch_bool_t create = (flags & NOTMUCH_FIND_CREATE);
*status_ret = NOTMUCH_STATUS_SUCCESS;
path = _notmuch_database_relative_path (notmuch, path);
-if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY)
+if (create && notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY)
INTERNAL_ERROR ("Failure to ensure database is writable");
-db = static_cast (notmuch->xapian_db);
-
directory = talloc (notmuch, notmuch_directory_t);
-if (unlikely (directory == NULL))
+if (unlikely (directory == NULL)) {
+ *status_ret = NOTMUCH_STATUS_OUT_OF_MEMORY;
return NULL;
+}
directory->notmuch = notmuch;
@@ -122,6 +135,13 @@ _notmuch_directory_create (notmuch_database_t *notmuch,
directory->document_id = directory->doc.get_docid ();
if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
+ if (!create) {
+ notmuch_directory_destroy (directory);
+ directory = NULL;
+ *status_ret = NOTMUCH_STATUS_SUCCESS;
+ goto DONE;
+ }
+
void *local = talloc_new (directory);
const char *parent, *basename;
Xapian::docid parent_id;
@@ -145,6 +165,8 @@ _notmuch_directory_create (notmuch_database_t *notmuch,
directory->doc.add_value (NOTMUCH_VALUE_TIMESTAMP,
Xapian::sortable_serialise (0));
+ db = static_cast (notmuch->xapian_db);
+
directory->document_id = _notmuch_database_generate_doc_id
(notmuch);
db->replace_document (directory->document_id, directory->doc);
talloc_free (local);
@@ -158,10 +180,11 @@ _notmuch_directory_create (notmuch_database_t *notmuch,
[PATCH 2/9] lib: Perform the same transformation to _notmuch_database_find_directory_id
Now _notmuch_database_find_directory_id takes a flags argument, which
it passes through to _notmuch_directory_create and can indicate if the
directory does not exist. Again, callers have been updated, but
retain their original behavior.
---
lib/database.cc | 14 +++---
lib/directory.cc |8 +++-
lib/notmuch-private.h |1 +
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index df996a9..716982d 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1197,9 +1197,17 @@ _notmuch_database_split_path (void *ctx,
return NOTMUCH_STATUS_SUCCESS;
}
+/* Find the document ID of the specified directory.
+ *
+ * If (flags & NOTMUCH_FIND_CREATE), a new directory document will be
+ * created if one does not exist for 'path'. Otherwise, if the
+ * directory document does not exist, this sets *directory_id to
+ * ((unsigned int)-1) and returns NOTMUCH_STATUS_SUCCESS.
+ */
notmuch_status_t
_notmuch_database_find_directory_id (notmuch_database_t *notmuch,
const char *path,
+notmuch_find_flags_t flags,
unsigned int *directory_id)
{
notmuch_directory_t *directory;
@@ -1210,8 +1218,8 @@ _notmuch_database_find_directory_id (notmuch_database_t
*notmuch,
return NOTMUCH_STATUS_SUCCESS;
}
-directory = _notmuch_directory_create (notmuch, path, NOTMUCH_FIND_CREATE,
&status);
-if (status) {
+directory = _notmuch_directory_create (notmuch, path, flags, &status);
+if (status || !directory) {
*directory_id = -1;
return status;
}
@@ -1260,7 +1268,7 @@ _notmuch_database_filename_to_direntry (void *ctx,
if (status)
return status;
-status = _notmuch_database_find_directory_id (notmuch, directory,
+status = _notmuch_database_find_directory_id (notmuch, directory,
NOTMUCH_FIND_CREATE,
&directory_id);
if (status)
return status;
diff --git a/lib/directory.cc b/lib/directory.cc
index 83bb19b..6a3ffed 100644
--- a/lib/directory.cc
+++ b/lib/directory.cc
@@ -153,7 +153,13 @@ _notmuch_directory_create (notmuch_database_t *notmuch,
_notmuch_database_split_path (local, path, &parent, &basename);
- _notmuch_database_find_directory_id (notmuch, parent, &parent_id);
+ *status_ret = _notmuch_database_find_directory_id (
+ notmuch, parent, NOTMUCH_FIND_CREATE, &parent_id);
+ if (*status_ret) {
+ notmuch_directory_destroy (directory);
+ directory = NULL;
+ goto DONE;
+ }
if (basename) {
term = talloc_asprintf (local, "%s%u:%s",
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 538274f..a36549d 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -195,6 +195,7 @@ _notmuch_database_find_unique_doc_id (notmuch_database_t
*notmuch,
notmuch_status_t
_notmuch_database_find_directory_id (notmuch_database_t *database,
const char *path,
+notmuch_find_flags_t flags,
unsigned int *directory_id);
const char *
--
1.7.10
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/9] lib: Make notmuch_database_get_directory return NULL if the directory is not found
Using the new support from _notmuch_directory_create, this makes
notmuch_database_get_directory a read-only operation that simply
returns the directory object if it exists or NULL otherwise. This
also means that notmuch_database_get_directory can work on read-only
databases.
This change breaks the directory mtime workaround in notmuch-new.c by
fixing the exact issue it was working around. This permits mtime
update races to prevent scans of changed directories, which
non-deterministically breaks a few tests. The next patch fixes this.
---
lib/database.cc |7 ++-
lib/notmuch-private.h |3 +++
lib/notmuch.h | 10 ++
3 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index b4c76b4..e27a0e1 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1328,12 +1328,9 @@ notmuch_database_get_directory (notmuch_database_t
*notmuch,
return NOTMUCH_STATUS_NULL_POINTER;
*directory = NULL;
-/* XXX Handle read-only databases properly */
-if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY)
- return NOTMUCH_STATUS_READ_ONLY_DATABASE;
-
try {
- *directory = _notmuch_directory_create (notmuch, path,
NOTMUCH_FIND_CREATE, &status);
+ *directory = _notmuch_directory_create (notmuch, path,
+ NOTMUCH_FIND_LOOKUP, &status);
} catch (const Xapian::Error &error) {
fprintf (stderr, "A Xapian exception occurred getting directory: %s.\n",
error.get_msg().c_str());
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 34f7ac7..bfb4111 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -148,6 +148,9 @@ typedef enum _notmuch_private_status {
/* Flags shared by various lookup functions. */
typedef enum _notmuch_find_flags {
+/* Lookup without creating any documents. This is the default
+ * behavior. */
+NOTMUCH_FIND_LOOKUP = 0,
/* If set, create the necessary document (or documents) if they
* are missing. Requires a read/write database. */
NOTMUCH_FIND_CREATE = 1<<0,
diff --git a/lib/notmuch.h b/lib/notmuch.h
index bbb17e4..3633bed 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -300,10 +300,8 @@ notmuch_database_end_atomic (notmuch_database_t *notmuch);
* (see notmuch_database_get_path), or else should be an absolute path
* with initial components that match the path of 'database'.
*
- * Note: Currently this will create the directory object if it doesn't
- * exist. In the future, when a directory object does not exist this
- * will return NOTMUCH_STATUS_SUCCESS and set *directory to NULL.
- * Callers should be prepared for this.
+ * If this directory object does not exist in the database, this
+ * returns NOTMUCH_STATUS_SUCCESS and sets *directory to NULL.
*
* Return value:
*
@@ -313,10 +311,6 @@ notmuch_database_end_atomic (notmuch_database_t *notmuch);
*
* NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred;
* directory not retrieved.
- *
- * NOTMUCH_STATUS_READ_ONLY_DATABASE: Database was opened in read-only
- * mode so the directory cannot be created (this case will be
- * removed in the future).
*/
notmuch_status_t
notmuch_database_get_directory (notmuch_database_t *database,
--
1.7.10
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 9/9] lib: Don't needlessly create directory docs in _notmuch_message_remove_filename
Previously, if passed a filename with a directory that did not exist in the database, _notmuch_message_remove_filename would needlessly create that directory document. Fix it so that doesn't happen. --- lib/message.cc |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/message.cc b/lib/message.cc index 8d552f1..6787506 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -541,8 +541,8 @@ _notmuch_message_remove_filename (notmuch_message_t *message, Xapian::TermIterator i, last; status = _notmuch_database_filename_to_direntry ( - local, message->notmuch, filename, NOTMUCH_FIND_CREATE, &direntry); -if (status) + local, message->notmuch, filename, NOTMUCH_FIND_LOOKUP, &direntry); +if (status || !direntry) return status; /* Unlink this file from its parent directory. */ -- 1.7.10 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 0/9] Fix directory lookup on read-only databases
This fixes notmuch_database_get_directory and notmuch_database_find_message_by_filename so that they don't attempt to create missing directory documents. This makes them work on read-only databases (and prevents n_d_f_m_b_f from crashing unceremoniously on read-only databases). Unfortunately, there are several functions involved in directory document lookup, so the first three patches simply add a creation flag at each necessary layer. The remaining patches then fix up the two API functions and their uses. If we do a 0.13.1 bug fix release, these patches could go in to complement the API changes made in 0.13 to support these fixes. David can make that call. There are several patches, but they're all short and incremental. bindings/python/notmuch/database.py | 11 --- lib/database.cc | 40 +--- lib/directory.cc| 41 +++-- lib/message.cc | 11 +-- lib/notmuch-private.h | 13 + lib/notmuch.h | 10 ++ notmuch-new.c | 36 +++- test/python |8 8 files changed, 95 insertions(+), 75 deletions(-) ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 6/9] python: Update Database.get_directory documentation
notmuch_database_get_directory no longer returns an error for read-only databases, so remove ReadOnlyDatabaseError from the list of get_directory exceptions. --- bindings/python/notmuch/database.py |3 --- 1 file changed, 3 deletions(-) diff --git a/bindings/python/notmuch/database.py b/bindings/python/notmuch/database.py index 797554d..ff89818 100644 --- a/bindings/python/notmuch/database.py +++ b/bindings/python/notmuch/database.py @@ -346,7 +346,6 @@ class Database(object): def get_directory(self, path): """Returns a :class:`Directory` of path, -(creating it if it does not exist(?)) :param path: An unicode string containing the path relative to the path of database (see :meth:`get_path`), or else should be an absolute @@ -354,8 +353,6 @@ class Database(object): :returns: :class:`Directory` or raises an exception. :raises: :exc:`FileError` if path is not relative database or absolute with initial components same as database. -:raises: :exc:`ReadOnlyDatabaseError` if the database has not been - opened in read-write mode """ self._assert_db_is_initialized() -- 1.7.10 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/9] lib: Perform the same transformation to _notmuch_database_filename_to_direntry
Now _notmuch_database_filename_to_direntry takes a flags argument and
can indicate if the necessary directory documents do not exist.
Again, callers have been updated, but retain their original behavior.
---
lib/database.cc | 17 +++--
lib/message.cc|9 -
lib/notmuch-private.h |1 +
3 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index 716982d..b4c76b4 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1248,13 +1248,16 @@ _notmuch_database_get_directory_path (void *ctx,
* database path), return a new string (with 'ctx' as the talloc
* owner) suitable for use as a direntry term value.
*
- * The necessary directory documents will be created in the database
- * as needed.
+ * If (flags & NOTMUCH_FIND_CREATE), the necessary directory documents
+ * will be created in the database as needed. Otherwise, if the
+ * necessary directory documents do not exist, this sets
+ * *direntry to NULL and returns NOTMUCH_STATUS_SUCCESS.
*/
notmuch_status_t
_notmuch_database_filename_to_direntry (void *ctx,
notmuch_database_t *notmuch,
const char *filename,
+ notmuch_find_flags_t flags,
char **direntry)
{
const char *relative, *directory, *basename;
@@ -1268,10 +1271,12 @@ _notmuch_database_filename_to_direntry (void *ctx,
if (status)
return status;
-status = _notmuch_database_find_directory_id (notmuch, directory,
NOTMUCH_FIND_CREATE,
+status = _notmuch_database_find_directory_id (notmuch, directory, flags,
&directory_id);
-if (status)
+if (status || directory_id == (unsigned int)-1) {
+ *direntry = NULL;
return status;
+}
*direntry = talloc_asprintf (ctx, "%u:%s", directory_id, basename);
@@ -1892,8 +1897,8 @@ notmuch_database_find_message_by_filename
(notmuch_database_t *notmuch,
local = talloc_new (notmuch);
try {
- status = _notmuch_database_filename_to_direntry (local, notmuch,
-filename, &direntry);
+ status = _notmuch_database_filename_to_direntry (
+ local, notmuch, filename, NOTMUCH_FIND_CREATE, &direntry);
if (status)
goto DONE;
diff --git a/lib/message.cc b/lib/message.cc
index 0075425..8d552f1 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -495,9 +495,8 @@ _notmuch_message_add_filename (notmuch_message_t *message,
if (status)
return status;
-status = _notmuch_database_filename_to_direntry (local,
-message->notmuch,
-filename, &direntry);
+status = _notmuch_database_filename_to_direntry (
+ local, message->notmuch, filename, NOTMUCH_FIND_CREATE, &direntry);
if (status)
return status;
@@ -541,8 +540,8 @@ _notmuch_message_remove_filename (notmuch_message_t
*message,
notmuch_status_t status;
Xapian::TermIterator i, last;
-status = _notmuch_database_filename_to_direntry (local, message->notmuch,
-filename, &direntry);
+status = _notmuch_database_filename_to_direntry (
+ local, message->notmuch, filename, NOTMUCH_FIND_CREATE, &direntry);
if (status)
return status;
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index a36549d..34f7ac7 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -207,6 +207,7 @@ notmuch_status_t
_notmuch_database_filename_to_direntry (void *ctx,
notmuch_database_t *notmuch,
const char *filename,
+ notmuch_find_flags_t flags,
char **direntry);
/* directory.cc */
--
1.7.10
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/6] cli: intialize crypto structure in show and reply
On May 17, 2012 5:26 PM, "Jameson Graef Rollins" wrote: > > On Thu, May 17 2012, Jani Nikula wrote: > > On Thu, 17 May 2012, Jameson Graef Rollins wrote: > >> This makes sure it has proper initialization values when it's created. > > > > Please don't do this. It's unnecessary; if one field is initialized with > > a designated initializer, the rest are initialized to zero (or NULL). > > It may be technically unnecessary, but why is that a reason to not do > it? I intentionally did this to make it clear what the defaults are. > Otherwise the defaults are essentially undefined, which is not good. > Maybe the structure initializes to the correct defaults, but why count > on that when we can set them to the correct default, and have it clear > to readers of the code? The values are not undefined, they are properly initialized, and we can count on it. For sure, not maybe. If you want to explicitly set them for clarity, it's a matter of taste. Personally I find it too verbose, but then again notmuch code is generally fairly verbose. If you insist on it, please at least drop the extra temp crypto variable, and initialize the struct in one initializer. BR, Jani. > > jamie. -- next part -- An HTML attachment was scrubbed... URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120517/789057d3/attachment-0001.html>
[PATCH 6/6] cli: lazily create the crypto gpg context only when needed
Quoth Jameson Graef Rollins on May 16 at 2:55 pm: > Move the creation of the crypto ctx into mime-node.c and create it > only when needed. This removes code duplication from notmuch-show and > notmuch-reply, and should speed up these functions considerably if the > crypto flags are provided but the messages don't have any > cryptographic parts. > --- > mime-node.c | 25 + > notmuch-reply.c | 19 --- > notmuch-show.c | 23 --- > 3 files changed, 25 insertions(+), 42 deletions(-) > > diff --git a/mime-node.c b/mime-node.c > index 8cdabc8..7278c74 100644 > --- a/mime-node.c > +++ b/mime-node.c > @@ -182,6 +182,31 @@ _mime_node_create (mime_node_t *parent, GMimeObject > *part) This patch should also update the documentation comment for mime_node_open.
[PATCH 4/6] cli: intialize crypto structure in show and reply
Quoth Jameson Graef Rollins on May 16 at 2:55 pm:
> This makes sure it has proper initialization values when it's created.
> ---
> notmuch-reply.c |5 -
> notmuch-show.c | 10 +-
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 6662adb..3c967a0 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -673,7 +673,10 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
> char *query_string;
> int opt_index, ret = 0;
> int (*reply_format_func)(void *ctx, notmuch_config_t *config,
> notmuch_query_t *query, notmuch_crypto_t *crypto, notmuch_bool_t reply_all);
> -notmuch_crypto_t crypto = { .decrypt = FALSE };
> +notmuch_crypto_t crypto = {
> + .decrypt = FALSE,
> + .gpgctx = NULL,
> +};
> int format = FORMAT_DEFAULT;
> int reply_all = TRUE;
>
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 8b4d308..c606333 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -983,7 +983,15 @@ notmuch_show_command (void *ctx, unused (int argc),
> unused (char *argv[]))
> char *query_string;
> int opt_index, ret;
> const notmuch_show_format_t *format = &format_text;
> -notmuch_show_params_t params = { .part = -1, .omit_excluded = TRUE };
> +notmuch_crypto_t crypto = {
> + .decrypt = FALSE,
> + .gpgctx = NULL,
> +};
> +notmuch_show_params_t params = {
> + .part = -1,
> + .omit_excluded = TRUE,
> + .crypto = crypto,
> +};
You can omit the temporary variable and avoid the struct copy by doing
something like this:
notmuch_show_params_t params = {
.part = -1,
.omit_excluded = TRUE,
.crypto = {
.decrypt = FALSE,
.gpgctx = NULL,
},
};
> int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
> notmuch_bool_t verify = FALSE;
> int exclude = EXCLUDE_TRUE;
[PATCH 3/6] cli: modify mime_node_open to take crypto struct as argument
Quoth Jameson Graef Rollins on May 16 at 2:55 pm:
> Again, for interface simplification and getting rid of more #ifdefs.
> ---
> mime-node.c | 10 ++
> notmuch-client.h | 14 +-
> notmuch-reply.c |6 ++
> notmuch-show.c |3 +--
> 4 files changed, 10 insertions(+), 23 deletions(-)
>
> diff --git a/mime-node.c b/mime-node.c
> index 79a3654..4faeffc 100644
> --- a/mime-node.c
> +++ b/mime-node.c
> @@ -56,12 +56,7 @@ _mime_node_context_free (mime_node_context_t *res)
>
> notmuch_status_t
> mime_node_open (const void *ctx, notmuch_message_t *message,
> -#ifdef GMIME_ATLEAST_26
> - GMimeCryptoContext *cryptoctx,
> -#else
> - GMimeCipherContext *cryptoctx,
> -#endif
> - notmuch_bool_t decrypt, mime_node_t **root_out)
> + notmuch_crypto_t *crypto, mime_node_t **root_out)
> {
> const char *filename = notmuch_message_get_filename (message);
> mime_node_context_t *mctx;
> @@ -113,8 +108,7 @@ mime_node_open (const void *ctx, notmuch_message_t
> *message,
> goto DONE;
> }
>
> -mctx->crypto.gpgctx = cryptoctx;
> -mctx->crypto.decrypt = decrypt;
> +mctx->crypto = *crypto;
I think you want to store the notmuch_crypto_t* pointer in the
mime_node_t, rather than copying the value. This doesn't matter so
much at this point, but when you later start lazily constructing the
cyrpto context, storing it by value will force you to lazily
initialize it separately potentially for every mime_node_t instance.
>
> /* Create the root node */
> root->part = GMIME_OBJECT (mctx->mime_message);
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 2ad24cf..d86fab3 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -345,9 +345,10 @@ struct mime_node {
> };
>
> /* Construct a new MIME node pointing to the root message part of
> - * message. If cryptoctx is non-NULL, it will be used to verify
> - * signatures on any child parts. If decrypt is true, then cryptoctx
> - * will additionally be used to decrypt any encrypted child parts.
> + * message. If crypto.gpgctx is non-NULL, it will be used to verify
> + * signatures on any child parts. If crypto.decrypt is true, then
> + * crypto.gpgctx will additionally be used to decrypt any encrypted
> + * child parts.
> *
> * Return value:
> *
> @@ -359,12 +360,7 @@ struct mime_node {
> */
> notmuch_status_t
> mime_node_open (const void *ctx, notmuch_message_t *message,
> -#ifdef GMIME_ATLEAST_26
> - GMimeCryptoContext *cryptoctx,
> -#else
> - GMimeCipherContext *cryptoctx,
> -#endif
> - notmuch_bool_t decrypt, mime_node_t **node_out);
> + notmuch_crypto_t *crypto, mime_node_t **node_out);
>
> /* Return a new MIME node for the requested child part of parent.
> * parent will be used as the talloc context for the returned child
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index ed87899..6662adb 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -544,8 +544,7 @@ notmuch_reply_format_default(void *ctx,
> g_object_unref (G_OBJECT (reply));
> reply = NULL;
>
> - if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> - &root) == NOTMUCH_STATUS_SUCCESS) {
> + if (mime_node_open (ctx, message, crypto, &root) ==
> NOTMUCH_STATUS_SUCCESS) {
> format_part_reply (root);
> talloc_free (root);
> }
> @@ -574,8 +573,7 @@ notmuch_reply_format_json(void *ctx,
>
> messages = notmuch_query_search_messages (query);
> message = notmuch_messages_get (messages);
> -if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> - &node) != NOTMUCH_STATUS_SUCCESS)
> +if (mime_node_open (ctx, message, crypto, &node) !=
> NOTMUCH_STATUS_SUCCESS)
> return 1;
>
> reply = create_reply_message (ctx, config, message, reply_all);
> diff --git a/notmuch-show.c b/notmuch-show.c
> index d254179..8b4d308 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -810,8 +810,7 @@ show_message (void *ctx,
> mime_node_t *root, *part;
> notmuch_status_t status;
>
> -status = mime_node_open (local, message, params->crypto.gpgctx,
> - params->crypto.decrypt, &root);
> +status = mime_node_open (local, message, &(params->crypto), &root);
> if (status)
> goto DONE;
> part = mime_node_seek_dfs (root, (params->part < 0 ? 0 : params->part));
[PATCH 4/6] cli: intialize crypto structure in show and reply
On 05/17/2012 12:45 PM, Jameson Graef Rollins wrote: > I want them explicitly set for clarity, as well as safety. Code is > meant to be read by humans, not computers. I sympathize with this sentiment. > It's much safer to explicitly set them to what you want > them to be rather than just assume they'll be set correctly. I don't think it's an assumption -- Jani is probably relying on the C standard. Consider, for example, C99 [0]'s section 6.7.8.19, which says: all subobjects that are not initialized explicitly shall be initialized implicitly the same as objects that have static storage duration. the latter clause references 6.7.8.10, which says: If an object that has static storage duration is not initialized explicitly, then: ? if it has pointer type, it is initialized to a null pointer; ? if it has arithmetic type, it is initialized to (positive or unsigned) zero; ? if it is an aggregate, every member is initialized (recursively) according to these rules; So it's not just "an assumption", it's a guarantee from the underlying language standard. That said, it's a guarantee i was unaware of until i researched this. I'm certainly not a C guru, but i've internalized a fair amount of C's rules and structure and i'd never heard of this subobject-default-initialization-when-other-subobjects-are-initialized rule before. If i'd seen the uninitialized members of the struct, and that they were being used without explicit initialization, i would have had to do a bit of digging to understand what's happening. The real tradeoff in this choice is whether we prefer: a) more compact code to facilitate quick reading by experts or b) more verbose code to facilitate comprehension by the non-expert. I started this discussion leaning strongly toward the (b) perspective. But now that i know the relevant bits of the standard, i can sympathize with the (a) perspective as well :P Overall, i think i'm still in the (b) camp. But i think it's more important that we don't allow dithering over this issue to prevent the inclusion of this patch series, which is a step in the right direction for handling S/MIME messages as well as PGP/MIME. --dkg PS gcc's -pedantic argument provides the following warning: error: ISO C90 forbids specifying subobject to initialize So we probably want to specify -std=c99 at least to ensure our choice of subobject initialization is respected. [0] http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 1030 bytes Desc: OpenPGP digital signature URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120517/59f634ea/attachment.pgp>
[alot] announcing v0.3.1
Hi everyone! I have just released alot v0.3.1; You can get a tarball here [0]. This is mainly a bugfix release, the most notable addition feature-wise is the ability to sign outgoing messages via PGP/MIME. Cheers to Michael for this one! Detailed usage updates since v0.3: * use separate database for each write-queue entry when flushing * fix behaviour of editor spawning * fix opening of attachments in thread buffer * fix pre_edit_translate hook * fix opening of attachments without filename Content-Disposition parm * clean up and complete theming (bindings help/envelope/mainframe body) * fix datetime decoding issues * fix abort commands on pre-hook exceptions * fix correct default sendmail command to 'sendmail -t' * use '> ' instead of '>' to quote in replies/fwds * fix path completer wrt spaces in paths * fix UI when no buffers are open * fix issue with buffer type changing between flushes * support multiple addresses per abook contact when using 'abook' completer * smarter timestamp pretty printer * new hook 'timestamp_format' * merge multiple cc/to headers into one when displaying * respect NOTMUCH_CONFIG env var * smarter parsing of edited header values * allow for singleton lists without trailing comma in config * fix reverse-date sorted content in threadline if displayed * emacs-style C-a and C-E in prompts * added ability to sign outgoing mails via PGP/MIME Thanks goes to those who contributed: $ git shortlog -s -n 0.3.. |tail -n +2 24 Michael Stapelberg 5 Jakob ..everyone who send feedback and of course to all notmuch contributors. With David Bremners help I have started hacking together some metadata to build a debian package. This can be found in branch 'debian' but I presume there are still some things to be done on that (manpage generation..). As always, don't hesitate to send feedback, bug reports, feature or pull requests via the projects github page [1]. Cheers, /p [0]: https://github.com/pazz/alot/tarball/0.3.1 [1]: https://github.com/pazz/alot -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: signature URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120517/1bb9f576/attachment.pgp>
Re: [PATCH 6/6] cli: lazily create the crypto gpg context only when needed
Quoth Jameson Graef Rollins on May 16 at 2:55 pm: > Move the creation of the crypto ctx into mime-node.c and create it > only when needed. This removes code duplication from notmuch-show and > notmuch-reply, and should speed up these functions considerably if the > crypto flags are provided but the messages don't have any > cryptographic parts. > --- > mime-node.c | 25 + > notmuch-reply.c | 19 --- > notmuch-show.c | 23 --- > 3 files changed, 25 insertions(+), 42 deletions(-) > > diff --git a/mime-node.c b/mime-node.c > index 8cdabc8..7278c74 100644 > --- a/mime-node.c > +++ b/mime-node.c > @@ -182,6 +182,31 @@ _mime_node_create (mime_node_t *parent, GMimeObject > *part) This patch should also update the documentation comment for mime_node_open. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 4/6] cli: intialize crypto structure in show and reply
Quoth Jameson Graef Rollins on May 16 at 2:55 pm:
> This makes sure it has proper initialization values when it's created.
> ---
> notmuch-reply.c |5 -
> notmuch-show.c | 10 +-
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 6662adb..3c967a0 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -673,7 +673,10 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
> char *query_string;
> int opt_index, ret = 0;
> int (*reply_format_func)(void *ctx, notmuch_config_t *config,
> notmuch_query_t *query, notmuch_crypto_t *crypto, notmuch_bool_t reply_all);
> -notmuch_crypto_t crypto = { .decrypt = FALSE };
> +notmuch_crypto_t crypto = {
> + .decrypt = FALSE,
> + .gpgctx = NULL,
> +};
> int format = FORMAT_DEFAULT;
> int reply_all = TRUE;
>
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 8b4d308..c606333 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -983,7 +983,15 @@ notmuch_show_command (void *ctx, unused (int argc),
> unused (char *argv[]))
> char *query_string;
> int opt_index, ret;
> const notmuch_show_format_t *format = &format_text;
> -notmuch_show_params_t params = { .part = -1, .omit_excluded = TRUE };
> +notmuch_crypto_t crypto = {
> + .decrypt = FALSE,
> + .gpgctx = NULL,
> +};
> +notmuch_show_params_t params = {
> + .part = -1,
> + .omit_excluded = TRUE,
> + .crypto = crypto,
> +};
You can omit the temporary variable and avoid the struct copy by doing
something like this:
notmuch_show_params_t params = {
.part = -1,
.omit_excluded = TRUE,
.crypto = {
.decrypt = FALSE,
.gpgctx = NULL,
},
};
> int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
> notmuch_bool_t verify = FALSE;
> int exclude = EXCLUDE_TRUE;
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/6] cli: modify mime_node_open to take crypto struct as argument
Quoth Jameson Graef Rollins on May 16 at 2:55 pm:
> Again, for interface simplification and getting rid of more #ifdefs.
> ---
> mime-node.c | 10 ++
> notmuch-client.h | 14 +-
> notmuch-reply.c |6 ++
> notmuch-show.c |3 +--
> 4 files changed, 10 insertions(+), 23 deletions(-)
>
> diff --git a/mime-node.c b/mime-node.c
> index 79a3654..4faeffc 100644
> --- a/mime-node.c
> +++ b/mime-node.c
> @@ -56,12 +56,7 @@ _mime_node_context_free (mime_node_context_t *res)
>
> notmuch_status_t
> mime_node_open (const void *ctx, notmuch_message_t *message,
> -#ifdef GMIME_ATLEAST_26
> - GMimeCryptoContext *cryptoctx,
> -#else
> - GMimeCipherContext *cryptoctx,
> -#endif
> - notmuch_bool_t decrypt, mime_node_t **root_out)
> + notmuch_crypto_t *crypto, mime_node_t **root_out)
> {
> const char *filename = notmuch_message_get_filename (message);
> mime_node_context_t *mctx;
> @@ -113,8 +108,7 @@ mime_node_open (const void *ctx, notmuch_message_t
> *message,
> goto DONE;
> }
>
> -mctx->crypto.gpgctx = cryptoctx;
> -mctx->crypto.decrypt = decrypt;
> +mctx->crypto = *crypto;
I think you want to store the notmuch_crypto_t* pointer in the
mime_node_t, rather than copying the value. This doesn't matter so
much at this point, but when you later start lazily constructing the
cyrpto context, storing it by value will force you to lazily
initialize it separately potentially for every mime_node_t instance.
>
> /* Create the root node */
> root->part = GMIME_OBJECT (mctx->mime_message);
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 2ad24cf..d86fab3 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -345,9 +345,10 @@ struct mime_node {
> };
>
> /* Construct a new MIME node pointing to the root message part of
> - * message. If cryptoctx is non-NULL, it will be used to verify
> - * signatures on any child parts. If decrypt is true, then cryptoctx
> - * will additionally be used to decrypt any encrypted child parts.
> + * message. If crypto.gpgctx is non-NULL, it will be used to verify
> + * signatures on any child parts. If crypto.decrypt is true, then
> + * crypto.gpgctx will additionally be used to decrypt any encrypted
> + * child parts.
> *
> * Return value:
> *
> @@ -359,12 +360,7 @@ struct mime_node {
> */
> notmuch_status_t
> mime_node_open (const void *ctx, notmuch_message_t *message,
> -#ifdef GMIME_ATLEAST_26
> - GMimeCryptoContext *cryptoctx,
> -#else
> - GMimeCipherContext *cryptoctx,
> -#endif
> - notmuch_bool_t decrypt, mime_node_t **node_out);
> + notmuch_crypto_t *crypto, mime_node_t **node_out);
>
> /* Return a new MIME node for the requested child part of parent.
> * parent will be used as the talloc context for the returned child
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index ed87899..6662adb 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -544,8 +544,7 @@ notmuch_reply_format_default(void *ctx,
> g_object_unref (G_OBJECT (reply));
> reply = NULL;
>
> - if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> - &root) == NOTMUCH_STATUS_SUCCESS) {
> + if (mime_node_open (ctx, message, crypto, &root) ==
> NOTMUCH_STATUS_SUCCESS) {
> format_part_reply (root);
> talloc_free (root);
> }
> @@ -574,8 +573,7 @@ notmuch_reply_format_json(void *ctx,
>
> messages = notmuch_query_search_messages (query);
> message = notmuch_messages_get (messages);
> -if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> - &node) != NOTMUCH_STATUS_SUCCESS)
> +if (mime_node_open (ctx, message, crypto, &node) !=
> NOTMUCH_STATUS_SUCCESS)
> return 1;
>
> reply = create_reply_message (ctx, config, message, reply_all);
> diff --git a/notmuch-show.c b/notmuch-show.c
> index d254179..8b4d308 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -810,8 +810,7 @@ show_message (void *ctx,
> mime_node_t *root, *part;
> notmuch_status_t status;
>
> -status = mime_node_open (local, message, params->crypto.gpgctx,
> - params->crypto.decrypt, &root);
> +status = mime_node_open (local, message, &(params->crypto), &root);
> if (status)
> goto DONE;
> part = mime_node_seek_dfs (root, (params->part < 0 ? 0 : params->part));
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 4/6] cli: intialize crypto structure in show and reply
On 05/17/2012 12:45 PM, Jameson Graef Rollins wrote: > I want them explicitly set for clarity, as well as safety. Code is > meant to be read by humans, not computers. I sympathize with this sentiment. > It's much safer to explicitly set them to what you want > them to be rather than just assume they'll be set correctly. I don't think it's an assumption -- Jani is probably relying on the C standard. Consider, for example, C99 [0]'s section 6.7.8.19, which says: all subobjects that are not initialized explicitly shall be initialized implicitly the same as objects that have static storage duration. the latter clause references 6.7.8.10, which says: If an object that has static storage duration is not initialized explicitly, then: — if it has pointer type, it is initialized to a null pointer; — if it has arithmetic type, it is initialized to (positive or unsigned) zero; — if it is an aggregate, every member is initialized (recursively) according to these rules; So it's not just "an assumption", it's a guarantee from the underlying language standard. That said, it's a guarantee i was unaware of until i researched this. I'm certainly not a C guru, but i've internalized a fair amount of C's rules and structure and i'd never heard of this subobject-default-initialization-when-other-subobjects-are-initialized rule before. If i'd seen the uninitialized members of the struct, and that they were being used without explicit initialization, i would have had to do a bit of digging to understand what's happening. The real tradeoff in this choice is whether we prefer: a) more compact code to facilitate quick reading by experts or b) more verbose code to facilitate comprehension by the non-expert. I started this discussion leaning strongly toward the (b) perspective. But now that i know the relevant bits of the standard, i can sympathize with the (a) perspective as well :P Overall, i think i'm still in the (b) camp. But i think it's more important that we don't allow dithering over this issue to prevent the inclusion of this patch series, which is a step in the right direction for handling S/MIME messages as well as PGP/MIME. --dkg PS gcc's -pedantic argument provides the following warning: error: ISO C90 forbids specifying subobject to initialize So we probably want to specify -std=c99 at least to ensure our choice of subobject initialization is respected. [0] http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf signature.asc Description: OpenPGP digital signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [alot] announcing v0.3.1
Quoting Jameson Graef Rollins (2012-05-17 16:42:53) > On Thu, May 17 2012, Patrick Totzke wrote: > > With David Bremners help I have started hacking together some metadata to > > build > > a debian package. This can be found in branch 'debian' but I presume there > > are still > > some things to be done on that (manpage generation..). > > Hi, Patrick. I put together some Debian packaging a while back that I > never did anything with: > > git://finestructure.net/alot [debian] > > It's a bit old, but it's mostly complete. Hopefully you'll find it > helpful. Hey Jamie, Ah yes, I forgot about this. Thanks for reminding me! IIRC, I could not get this to "run through" on my machine but thats surely due to my limited knowledge about debian packaging. I'll have a look at this again and compare it with my stuff. Cheers, /p signature.asc Description: signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 4/6] cli: intialize crypto structure in show and reply
On Thu, May 17 2012, Jani Nikula wrote: > In short, when you read enough code, having everything explicitly > stated becomes a burden. It's explicit, but you have to read it all, > even when there really is no need to. Not everyone who reads the code is an expert. I think it's important for the code to be clear to everyone. Experts can more easily gloss over the trivial parts than novices can divine things that are hidden. jamie. pgp3Un0fY8kTJ.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/6] cli: intialize crypto structure in show and reply
On Thu, May 17 2012, Jani Nikula wrote: > In short, when you read enough code, having everything explicitly > stated becomes a burden. It's explicit, but you have to read it all, > even when there really is no need to. Not everyone who reads the code is an expert. I think it's important for the code to be clear to everyone. Experts can more easily gloss over the trivial parts than novices can divine things that are hidden. jamie. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 835 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120517/cfc5b5c4/attachment.pgp>
Re: [PATCH 4/6] cli: intialize crypto structure in show and reply
On Thu, 17 May 2012, Jameson Graef Rollins wrote: > On Thu, May 17 2012, Jani Nikula wrote: >> The values are not undefined, they are properly initialized, and we can >> count on it. For sure, not maybe. If you want to explicitly set them for >> clarity, it's a matter of taste. Personally I find it too verbose, but then >> again notmuch code is generally fairly verbose. > > I want them explicitly set for clarity, as well as safety. Code is > meant to be read by humans, not computers. Brevity is not always a > virtue if it sacrifices clarity. It's much nicer to have the defaults > clearly stated in the initialization, than to force the reader to > understand how the initialization works and to interpret what that means > for the current case. I also don't think it's safe to assume that the > variables will be always be "properly" initialized in your favor in > perpetuity. It's much safer to explicitly set them to what you want > them to be rather than just assume they'll be set correctly. In short, when you read enough code, having everything explicitly stated becomes a burden. It's explicit, but you have to read it all, even when there really is no need to. >> If you insist on it, please at least drop the extra temp crypto >> variable, and initialize the struct in one initializer. > > I don't see why this matters either. Again, I think this is just a > matter of taste. I would rather the code be verbose where clarity > requires it, rather than always trying to make the code as terse as > possible. You introduce an extra variable that every reader of your code has to track down to realize that it's only ever used once to initialize another variable. You make code harder for other people to read. I have now offered my review and opinions on the matter; I will not pursue this discussion further. BR, Jani. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/3] NEWS: Insert markdown formatting commands in 0.13 section text
On Thu, May 17 2012, David Bremner wrote: > Tomi Ollila writes: >> >> id:"1337152910-22371-4-git-send-email-tomi.oll...@iki.fi" >> id:"1337152910-22371-5-git-send-email-tomi.oll...@iki.fi" >> id:"1337152910-22371-6-git-send-email-tomi.oll...@iki.fi" >> id:"1337152910-22371-7-git-send-email-tomi.oll...@iki.fi" > > I pushed those to release. I'll merge them to master after a bit; > hopefully we'll figure out that reply encoding thing and do a bug fix > release. I got it working by doing this: --8<8<8<8<8<8<8<8<8<8<8<8<-- diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index 7fa441a..7523bb6 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -244,7 +244,9 @@ the given type." current buffer, if possible." (let ((display-buffer (current-buffer))) (with-temp-buffer - (let* ((charset (plist-get part :content-charset)) + ;; (let* ((charset (or (plist-get part :content-charset) "utf-8")) + ;; (let* ((charset (or (plist-get part :content-charset) "latin-1")) + (let* ((charset (or (plist-get part :content-charset) 'gnus-decoded)) (handle (mm-make-handle (current-buffer) `(,content-type (charset . ,charset) ;; If the user wants the part inlined, insert the content and ;; test whether we are able to inline it (which includes both --8<8<8<8<8<8<8<8<8<8<8<8<-- (using "latin-1" also provided desired results... but with "utf-8" results were the same as with default 'nil) But, for consistency (with `notmuch show ...` handling), when content-type is "text/plain" (and as `notmuch reply --format=json` provides text/plain content inline) we should use the construct `(insert (notmuch-get-bodypart-content msg part nth ...)` to insert that content (as done before f6c170fabca8f39e74705e3813504137811bf162 and is also done in `notmuch-show-insert-part-text/plain`). If anyone is faster than me to provide patch what would be fine ;) > d Tomi ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/6] cli: intialize crypto structure in show and reply
On Thu, 17 May 2012, Jameson Graef Rollins
wrote:
> This makes sure it has proper initialization values when it's created.
Please don't do this. It's unnecessary; if one field is initialized with
a designated initializer, the rest are initialized to zero (or NULL).
BR,
Jani.
> ---
> notmuch-reply.c |5 -
> notmuch-show.c | 10 +-
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 6662adb..3c967a0 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -673,7 +673,10 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
> char *query_string;
> int opt_index, ret = 0;
> int (*reply_format_func)(void *ctx, notmuch_config_t *config,
> notmuch_query_t *query, notmuch_crypto_t *crypto, notmuch_bool_t reply_all);
> -notmuch_crypto_t crypto = { .decrypt = FALSE };
> +notmuch_crypto_t crypto = {
> + .decrypt = FALSE,
> + .gpgctx = NULL,
> +};
> int format = FORMAT_DEFAULT;
> int reply_all = TRUE;
>
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 8b4d308..c606333 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -983,7 +983,15 @@ notmuch_show_command (void *ctx, unused (int argc),
> unused (char *argv[]))
> char *query_string;
> int opt_index, ret;
> const notmuch_show_format_t *format = &format_text;
> -notmuch_show_params_t params = { .part = -1, .omit_excluded = TRUE };
> +notmuch_crypto_t crypto = {
> + .decrypt = FALSE,
> + .gpgctx = NULL,
> +};
> +notmuch_show_params_t params = {
> + .part = -1,
> + .omit_excluded = TRUE,
> + .crypto = crypto,
> +};
> int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
> notmuch_bool_t verify = FALSE;
> int exclude = EXCLUDE_TRUE;
> --
> 1.7.10
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/6] cli: modify mime_node_open to take crypto struct as argument
On Thu, 17 May 2012, Jameson Graef Rollins
wrote:
> Again, for interface simplification and getting rid of more #ifdefs.
> ---
> mime-node.c | 10 ++
> notmuch-client.h | 14 +-
> notmuch-reply.c |6 ++
> notmuch-show.c |3 +--
> 4 files changed, 10 insertions(+), 23 deletions(-)
>
> diff --git a/mime-node.c b/mime-node.c
> index 79a3654..4faeffc 100644
> --- a/mime-node.c
> +++ b/mime-node.c
> @@ -56,12 +56,7 @@ _mime_node_context_free (mime_node_context_t *res)
>
> notmuch_status_t
> mime_node_open (const void *ctx, notmuch_message_t *message,
> -#ifdef GMIME_ATLEAST_26
> - GMimeCryptoContext *cryptoctx,
> -#else
> - GMimeCipherContext *cryptoctx,
> -#endif
> - notmuch_bool_t decrypt, mime_node_t **root_out)
> + notmuch_crypto_t *crypto, mime_node_t **root_out)
> {
> const char *filename = notmuch_message_get_filename (message);
> mime_node_context_t *mctx;
> @@ -113,8 +108,7 @@ mime_node_open (const void *ctx, notmuch_message_t
> *message,
> goto DONE;
> }
>
> -mctx->crypto.gpgctx = cryptoctx;
> -mctx->crypto.decrypt = decrypt;
> +mctx->crypto = *crypto;
>
> /* Create the root node */
> root->part = GMIME_OBJECT (mctx->mime_message);
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 2ad24cf..d86fab3 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -345,9 +345,10 @@ struct mime_node {
> };
>
> /* Construct a new MIME node pointing to the root message part of
> - * message. If cryptoctx is non-NULL, it will be used to verify
> - * signatures on any child parts. If decrypt is true, then cryptoctx
> - * will additionally be used to decrypt any encrypted child parts.
> + * message. If crypto.gpgctx is non-NULL, it will be used to verify
> + * signatures on any child parts. If crypto.decrypt is true, then
> + * crypto.gpgctx will additionally be used to decrypt any encrypted
> + * child parts.
> *
> * Return value:
> *
> @@ -359,12 +360,7 @@ struct mime_node {
> */
> notmuch_status_t
> mime_node_open (const void *ctx, notmuch_message_t *message,
> -#ifdef GMIME_ATLEAST_26
> - GMimeCryptoContext *cryptoctx,
> -#else
> - GMimeCipherContext *cryptoctx,
> -#endif
> - notmuch_bool_t decrypt, mime_node_t **node_out);
> + notmuch_crypto_t *crypto, mime_node_t **node_out);
>
> /* Return a new MIME node for the requested child part of parent.
> * parent will be used as the talloc context for the returned child
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index ed87899..6662adb 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -544,8 +544,7 @@ notmuch_reply_format_default(void *ctx,
> g_object_unref (G_OBJECT (reply));
> reply = NULL;
>
> - if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> - &root) == NOTMUCH_STATUS_SUCCESS) {
> + if (mime_node_open (ctx, message, crypto, &root) ==
> NOTMUCH_STATUS_SUCCESS) {
> format_part_reply (root);
> talloc_free (root);
> }
> @@ -574,8 +573,7 @@ notmuch_reply_format_json(void *ctx,
>
> messages = notmuch_query_search_messages (query);
> message = notmuch_messages_get (messages);
> -if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> - &node) != NOTMUCH_STATUS_SUCCESS)
> +if (mime_node_open (ctx, message, crypto, &node) !=
> NOTMUCH_STATUS_SUCCESS)
> return 1;
>
> reply = create_reply_message (ctx, config, message, reply_all);
> diff --git a/notmuch-show.c b/notmuch-show.c
> index d254179..8b4d308 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -810,8 +810,7 @@ show_message (void *ctx,
> mime_node_t *root, *part;
> notmuch_status_t status;
>
> -status = mime_node_open (local, message, params->crypto.gpgctx,
> - params->crypto.decrypt, &root);
> +status = mime_node_open (local, message, &(params->crypto), &root);
The parenthesis around params->crypto are not needed, otherwise LGTM.
> if (status)
> goto DONE;
> part = mime_node_seek_dfs (root, (params->part < 0 ? 0 : params->part));
> --
> 1.7.10
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/6] cli: modify mime_node_context to use the new notmuch_crypto_t
On Thu, 17 May 2012, Jameson Graef Rollins
wrote:
> This simplifies some more interfaces and gets rid of another #ifdef.
LGTM.
> ---
> mime-node.c | 23 +--
> 1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/mime-node.c b/mime-node.c
> index a95bdab..79a3654 100644
> --- a/mime-node.c
> +++ b/mime-node.c
> @@ -33,12 +33,7 @@ typedef struct mime_node_context {
> GMimeMessage *mime_message;
>
> /* Context provided by the caller. */
> -#ifdef GMIME_ATLEAST_26
> -GMimeCryptoContext *cryptoctx;
> -#else
> -GMimeCipherContext *cryptoctx;
> -#endif
> -notmuch_bool_t decrypt;
> +notmuch_crypto_t crypto;
> } mime_node_context_t;
>
> static int
> @@ -118,8 +113,8 @@ mime_node_open (const void *ctx, notmuch_message_t
> *message,
> goto DONE;
> }
>
> -mctx->cryptoctx = cryptoctx;
> -mctx->decrypt = decrypt;
> +mctx->crypto.gpgctx = cryptoctx;
> +mctx->crypto.decrypt = decrypt;
>
> /* Create the root node */
> root->part = GMIME_OBJECT (mctx->mime_message);
> @@ -195,7 +190,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
>
> /* Handle PGP/MIME parts */
> if (GMIME_IS_MULTIPART_ENCRYPTED (part)
> - && node->ctx->cryptoctx && node->ctx->decrypt) {
> + && node->ctx->crypto.gpgctx && node->ctx->crypto.decrypt) {
> if (node->nchildren != 2) {
> /* this violates RFC 3156 section 4, so we won't bother with it. */
> fprintf (stderr, "Error: %d part(s) for a multipart/encrypted "
> @@ -208,10 +203,10 @@ _mime_node_create (mime_node_t *parent, GMimeObject
> *part)
> #ifdef GMIME_ATLEAST_26
> GMimeDecryptResult *decrypt_result = NULL;
> node->decrypted_child = g_mime_multipart_encrypted_decrypt
> - (encrypteddata, node->ctx->cryptoctx, &decrypt_result, &err);
> + (encrypteddata, node->ctx->crypto.gpgctx, &decrypt_result,
> &err);
> #else
> node->decrypted_child = g_mime_multipart_encrypted_decrypt
> - (encrypteddata, node->ctx->cryptoctx, &err);
> + (encrypteddata, node->ctx->crypto.gpgctx, &err);
> #endif
> if (node->decrypted_child) {
> node->decrypt_success = node->verify_attempted = TRUE;
> @@ -229,7 +224,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
>(err ? err->message : "no error explanation given"));
> }
> }
> -} else if (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->cryptoctx) {
> +} else if (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto.gpgctx)
> {
> if (node->nchildren != 2) {
> /* this violates RFC 3156 section 5, so we won't bother with it. */
> fprintf (stderr, "Error: %d part(s) for a multipart/signed message "
> @@ -238,7 +233,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
> } else {
> #ifdef GMIME_ATLEAST_26
> node->sig_list = g_mime_multipart_signed_verify
> - (GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);
> + (GMIME_MULTIPART_SIGNED (part), node->ctx->crypto.gpgctx, &err);
> node->verify_attempted = TRUE;
>
> if (!node->sig_list)
> @@ -254,7 +249,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
>* In GMime 2.6, they're both non-const, so we'll be able
>* to clean up this asymmetry. */
> GMimeSignatureValidity *sig_validity =
> g_mime_multipart_signed_verify
> - (GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);
> + (GMIME_MULTIPART_SIGNED (part), node->ctx->crypto.gpgctx, &err);
> node->verify_attempted = TRUE;
> node->sig_validity = sig_validity;
> if (sig_validity) {
> --
> 1.7.10
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/6] cli: new crypto structure to store crypto contexts and parameters
On Thu, 17 May 2012, Jameson Graef Rollins
wrote:
> The main point here is to keep track of the crypto stuff together in
> one place. In notmuch-show the crypto struct is a sub structure of
> the parameters struct. In notmuch-reply, which had been using a
> notmuch_show_params_t to store the crypto parameters, we can now just
> use the general crypto struct.
Looks good. My only (potentially unwarranted) worry is dropping the use
of notmuch_show_params_t in reply. This diverges the show/reply code,
making any future unification of them slightly harder. And if reply ever
needs params, we'll need to bring it back. But perhaps I worry too
much. :)
BR,
Jani.
>
> I slip in a name change of the crypto context itself to better reflect
> what the context is specifically for: it's actually a GPG context,
> which is a sub type of Crypto context. There are other types of
> Crypto contexts (Pkcs7 in particular) so we want to be clear.
>
> The following patches will use this to simplify some function
> interfaces.
> ---
> notmuch-client.h | 16 ++--
> notmuch-reply.c | 34 +-
> notmuch-show.c | 22 +++---
> 3 files changed, 38 insertions(+), 34 deletions(-)
>
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 19b7f01..2ad24cf 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -74,17 +74,21 @@ typedef struct notmuch_show_format {
> const char *message_set_end;
> } notmuch_show_format_t;
>
> +typedef struct notmuch_crypto {
> +#ifdef GMIME_ATLEAST_26
> +GMimeCryptoContext* gpgctx;
> +#else
> +GMimeCipherContext* gpgctx;
> +#endif
> +notmuch_bool_t decrypt;
> +} notmuch_crypto_t;
> +
> typedef struct notmuch_show_params {
> notmuch_bool_t entire_thread;
> notmuch_bool_t omit_excluded;
> notmuch_bool_t raw;
> int part;
> -#ifdef GMIME_ATLEAST_26
> -GMimeCryptoContext* cryptoctx;
> -#else
> -GMimeCipherContext* cryptoctx;
> -#endif
> -notmuch_bool_t decrypt;
> +notmuch_crypto_t crypto;
> } notmuch_show_params_t;
>
> /* There's no point in continuing when we've detected that we've done
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 7184a5d..ed87899 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -515,7 +515,7 @@ static int
> notmuch_reply_format_default(void *ctx,
>notmuch_config_t *config,
>notmuch_query_t *query,
> - notmuch_show_params_t *params,
> + notmuch_crypto_t *crypto,
>notmuch_bool_t reply_all)
> {
> GMimeMessage *reply;
> @@ -544,7 +544,7 @@ notmuch_reply_format_default(void *ctx,
> g_object_unref (G_OBJECT (reply));
> reply = NULL;
>
> - if (mime_node_open (ctx, message, params->cryptoctx, params->decrypt,
> + if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> &root) == NOTMUCH_STATUS_SUCCESS) {
> format_part_reply (root);
> talloc_free (root);
> @@ -559,7 +559,7 @@ static int
> notmuch_reply_format_json(void *ctx,
> notmuch_config_t *config,
> notmuch_query_t *query,
> - notmuch_show_params_t *params,
> + notmuch_crypto_t *crypto,
> notmuch_bool_t reply_all)
> {
> GMimeMessage *reply;
> @@ -574,7 +574,7 @@ notmuch_reply_format_json(void *ctx,
>
> messages = notmuch_query_search_messages (query);
> message = notmuch_messages_get (messages);
> -if (mime_node_open (ctx, message, params->cryptoctx, params->decrypt,
> +if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> &node) != NOTMUCH_STATUS_SUCCESS)
> return 1;
>
> @@ -605,7 +605,7 @@ static int
> notmuch_reply_format_headers_only(void *ctx,
> notmuch_config_t *config,
> notmuch_query_t *query,
> - unused (notmuch_show_params_t *params),
> + unused (notmuch_crypto_t *crypto),
> notmuch_bool_t reply_all)
> {
> GMimeMessage *reply;
> @@ -674,8 +674,8 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
> notmuch_query_t *query;
> char *query_string;
> int opt_index, ret = 0;
> -int (*reply_format_func)(void *ctx, notmuch_config_t *config,
> notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t
> reply_all);
> -notmuch_show_params_t params = { .part = -1 };
> +int (*reply_format_func)(void *ctx, notmuch_config_t *config,
> notmuch_query_t *query, notmuch_crypto_t *crypto, notmuch_bool_t reply_all);
> +notmuch_crypto_t crypto = { .decrypt = FALSE };
> int format = FORMAT_DEFAULT;
> int reply_all = TRUE;
>
> @@ -689,7 +689,7 @@ notmuch_reply_command (vo
Re: [PATCH 4/6] cli: intialize crypto structure in show and reply
On Thu, May 17 2012, Jani Nikula wrote: > The values are not undefined, they are properly initialized, and we can > count on it. For sure, not maybe. If you want to explicitly set them for > clarity, it's a matter of taste. Personally I find it too verbose, but then > again notmuch code is generally fairly verbose. I want them explicitly set for clarity, as well as safety. Code is meant to be read by humans, not computers. Brevity is not always a virtue if it sacrifices clarity. It's much nicer to have the defaults clearly stated in the initialization, than to force the reader to understand how the initialization works and to interpret what that means for the current case. I also don't think it's safe to assume that the variables will be always be "properly" initialized in your favor in perpetuity. It's much safer to explicitly set them to what you want them to be rather than just assume they'll be set correctly. > If you insist on it, please at least drop the extra temp crypto > variable, and initialize the struct in one initializer. I don't see why this matters either. Again, I think this is just a matter of taste. I would rather the code be verbose where clarity requires it, rather than always trying to make the code as terse as possible. jamie. pgp2cWfT5P1Tb.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/6] cli: intialize crypto structure in show and reply
On Thu, May 17 2012, Jani Nikula wrote: > The values are not undefined, they are properly initialized, and we can > count on it. For sure, not maybe. If you want to explicitly set them for > clarity, it's a matter of taste. Personally I find it too verbose, but then > again notmuch code is generally fairly verbose. I want them explicitly set for clarity, as well as safety. Code is meant to be read by humans, not computers. Brevity is not always a virtue if it sacrifices clarity. It's much nicer to have the defaults clearly stated in the initialization, than to force the reader to understand how the initialization works and to interpret what that means for the current case. I also don't think it's safe to assume that the variables will be always be "properly" initialized in your favor in perpetuity. It's much safer to explicitly set them to what you want them to be rather than just assume they'll be set correctly. > If you insist on it, please at least drop the extra temp crypto > variable, and initialize the struct in one initializer. I don't see why this matters either. Again, I think this is just a matter of taste. I would rather the code be verbose where clarity requires it, rather than always trying to make the code as terse as possible. jamie. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 835 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120517/2a557c17/attachment.pgp>
Re: [PATCH 4/6] cli: intialize crypto structure in show and reply
On May 17, 2012 5:26 PM, "Jameson Graef Rollins" wrote: > > On Thu, May 17 2012, Jani Nikula wrote: > > On Thu, 17 May 2012, Jameson Graef Rollins wrote: > >> This makes sure it has proper initialization values when it's created. > > > > Please don't do this. It's unnecessary; if one field is initialized with > > a designated initializer, the rest are initialized to zero (or NULL). > > It may be technically unnecessary, but why is that a reason to not do > it? I intentionally did this to make it clear what the defaults are. > Otherwise the defaults are essentially undefined, which is not good. > Maybe the structure initializes to the correct defaults, but why count > on that when we can set them to the correct default, and have it clear > to readers of the code? The values are not undefined, they are properly initialized, and we can count on it. For sure, not maybe. If you want to explicitly set them for clarity, it's a matter of taste. Personally I find it too verbose, but then again notmuch code is generally fairly verbose. If you insist on it, please at least drop the extra temp crypto variable, and initialize the struct in one initializer. BR, Jani. > > jamie. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [alot] announcing v0.3.1
On Thu, May 17 2012, Patrick Totzke wrote: > With David Bremners help I have started hacking together some metadata to > build > a debian package. This can be found in branch 'debian' but I presume there > are still > some things to be done on that (manpage generation..). Hi, Patrick. I put together some Debian packaging a while back that I never did anything with: git://finestructure.net/alot [debian] It's a bit old, but it's mostly complete. Hopefully you'll find it helpful. jamie. pgpbw0ToZik6C.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[alot] announcing v0.3.1
On Thu, May 17 2012, Patrick Totzke wrote: > With David Bremners help I have started hacking together some metadata to > build > a debian package. This can be found in branch 'debian' but I presume there > are still > some things to be done on that (manpage generation..). Hi, Patrick. I put together some Debian packaging a while back that I never did anything with: git://finestructure.net/alot [debian] It's a bit old, but it's mostly complete. Hopefully you'll find it helpful. jamie. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 835 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120517/62badad1/attachment.pgp>
[alot] announcing v0.3.1
Hi everyone! I have just released alot v0.3.1; You can get a tarball here [0]. This is mainly a bugfix release, the most notable addition feature-wise is the ability to sign outgoing messages via PGP/MIME. Cheers to Michael for this one! Detailed usage updates since v0.3: * use separate database for each write-queue entry when flushing * fix behaviour of editor spawning * fix opening of attachments in thread buffer * fix pre_edit_translate hook * fix opening of attachments without filename Content-Disposition parm * clean up and complete theming (bindings help/envelope/mainframe body) * fix datetime decoding issues * fix abort commands on pre-hook exceptions * fix correct default sendmail command to 'sendmail -t' * use '> ' instead of '>' to quote in replies/fwds * fix path completer wrt spaces in paths * fix UI when no buffers are open * fix issue with buffer type changing between flushes * support multiple addresses per abook contact when using 'abook' completer * smarter timestamp pretty printer * new hook 'timestamp_format' * merge multiple cc/to headers into one when displaying * respect NOTMUCH_CONFIG env var * smarter parsing of edited header values * allow for singleton lists without trailing comma in config * fix reverse-date sorted content in threadline if displayed * emacs-style C-a and C-E in prompts * added ability to sign outgoing mails via PGP/MIME Thanks goes to those who contributed: $ git shortlog -s -n 0.3.. |tail -n +2 24 Michael Stapelberg 5 Jakob ..everyone who send feedback and of course to all notmuch contributors. With David Bremners help I have started hacking together some metadata to build a debian package. This can be found in branch 'debian' but I presume there are still some things to be done on that (manpage generation..). As always, don't hesitate to send feedback, bug reports, feature or pull requests via the projects github page [1]. Cheers, /p [0]: https://github.com/pazz/alot/tarball/0.3.1 [1]: https://github.com/pazz/alot signature.asc Description: signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 4/6] cli: intialize crypto structure in show and reply
On Thu, May 17 2012, Jani Nikula wrote: > On Thu, 17 May 2012, Jameson Graef Rollins wrote: >> This makes sure it has proper initialization values when it's created. > > Please don't do this. It's unnecessary; if one field is initialized with > a designated initializer, the rest are initialized to zero (or NULL). It may be technically unnecessary, but why is that a reason to not do it? I intentionally did this to make it clear what the defaults are. Otherwise the defaults are essentially undefined, which is not good. Maybe the structure initializes to the correct defaults, but why count on that when we can set them to the correct default, and have it clear to readers of the code? jamie. pgpX1NFOEAHqa.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/6] cli: intialize crypto structure in show and reply
On Thu, May 17 2012, Jani Nikula wrote: > On Thu, 17 May 2012, Jameson Graef Rollins > wrote: >> This makes sure it has proper initialization values when it's created. > > Please don't do this. It's unnecessary; if one field is initialized with > a designated initializer, the rest are initialized to zero (or NULL). It may be technically unnecessary, but why is that a reason to not do it? I intentionally did this to make it clear what the defaults are. Otherwise the defaults are essentially undefined, which is not good. Maybe the structure initializes to the correct defaults, but why count on that when we can set them to the correct default, and have it clear to readers of the code? jamie. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 835 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120517/a329ef95/attachment.pgp>
Re: [PATCH 4/6] cli: intialize crypto structure in show and reply
On Thu, 17 May 2012, Jameson Graef Rollins wrote:
> This makes sure it has proper initialization values when it's created.
Please don't do this. It's unnecessary; if one field is initialized with
a designated initializer, the rest are initialized to zero (or NULL).
BR,
Jani.
> ---
> notmuch-reply.c |5 -
> notmuch-show.c | 10 +-
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 6662adb..3c967a0 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -673,7 +673,10 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
> char *query_string;
> int opt_index, ret = 0;
> int (*reply_format_func)(void *ctx, notmuch_config_t *config,
> notmuch_query_t *query, notmuch_crypto_t *crypto, notmuch_bool_t reply_all);
> -notmuch_crypto_t crypto = { .decrypt = FALSE };
> +notmuch_crypto_t crypto = {
> + .decrypt = FALSE,
> + .gpgctx = NULL,
> +};
> int format = FORMAT_DEFAULT;
> int reply_all = TRUE;
>
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 8b4d308..c606333 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -983,7 +983,15 @@ notmuch_show_command (void *ctx, unused (int argc),
> unused (char *argv[]))
> char *query_string;
> int opt_index, ret;
> const notmuch_show_format_t *format = &format_text;
> -notmuch_show_params_t params = { .part = -1, .omit_excluded = TRUE };
> +notmuch_crypto_t crypto = {
> + .decrypt = FALSE,
> + .gpgctx = NULL,
> +};
> +notmuch_show_params_t params = {
> + .part = -1,
> + .omit_excluded = TRUE,
> + .crypto = crypto,
> +};
> int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
> notmuch_bool_t verify = FALSE;
> int exclude = EXCLUDE_TRUE;
> --
> 1.7.10
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/6] cli: modify mime_node_open to take crypto struct as argument
On Thu, 17 May 2012, Jameson Graef Rollins wrote:
> Again, for interface simplification and getting rid of more #ifdefs.
> ---
> mime-node.c | 10 ++
> notmuch-client.h | 14 +-
> notmuch-reply.c |6 ++
> notmuch-show.c |3 +--
> 4 files changed, 10 insertions(+), 23 deletions(-)
>
> diff --git a/mime-node.c b/mime-node.c
> index 79a3654..4faeffc 100644
> --- a/mime-node.c
> +++ b/mime-node.c
> @@ -56,12 +56,7 @@ _mime_node_context_free (mime_node_context_t *res)
>
> notmuch_status_t
> mime_node_open (const void *ctx, notmuch_message_t *message,
> -#ifdef GMIME_ATLEAST_26
> - GMimeCryptoContext *cryptoctx,
> -#else
> - GMimeCipherContext *cryptoctx,
> -#endif
> - notmuch_bool_t decrypt, mime_node_t **root_out)
> + notmuch_crypto_t *crypto, mime_node_t **root_out)
> {
> const char *filename = notmuch_message_get_filename (message);
> mime_node_context_t *mctx;
> @@ -113,8 +108,7 @@ mime_node_open (const void *ctx, notmuch_message_t
> *message,
> goto DONE;
> }
>
> -mctx->crypto.gpgctx = cryptoctx;
> -mctx->crypto.decrypt = decrypt;
> +mctx->crypto = *crypto;
>
> /* Create the root node */
> root->part = GMIME_OBJECT (mctx->mime_message);
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 2ad24cf..d86fab3 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -345,9 +345,10 @@ struct mime_node {
> };
>
> /* Construct a new MIME node pointing to the root message part of
> - * message. If cryptoctx is non-NULL, it will be used to verify
> - * signatures on any child parts. If decrypt is true, then cryptoctx
> - * will additionally be used to decrypt any encrypted child parts.
> + * message. If crypto.gpgctx is non-NULL, it will be used to verify
> + * signatures on any child parts. If crypto.decrypt is true, then
> + * crypto.gpgctx will additionally be used to decrypt any encrypted
> + * child parts.
> *
> * Return value:
> *
> @@ -359,12 +360,7 @@ struct mime_node {
> */
> notmuch_status_t
> mime_node_open (const void *ctx, notmuch_message_t *message,
> -#ifdef GMIME_ATLEAST_26
> - GMimeCryptoContext *cryptoctx,
> -#else
> - GMimeCipherContext *cryptoctx,
> -#endif
> - notmuch_bool_t decrypt, mime_node_t **node_out);
> + notmuch_crypto_t *crypto, mime_node_t **node_out);
>
> /* Return a new MIME node for the requested child part of parent.
> * parent will be used as the talloc context for the returned child
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index ed87899..6662adb 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -544,8 +544,7 @@ notmuch_reply_format_default(void *ctx,
> g_object_unref (G_OBJECT (reply));
> reply = NULL;
>
> - if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> - &root) == NOTMUCH_STATUS_SUCCESS) {
> + if (mime_node_open (ctx, message, crypto, &root) ==
> NOTMUCH_STATUS_SUCCESS) {
> format_part_reply (root);
> talloc_free (root);
> }
> @@ -574,8 +573,7 @@ notmuch_reply_format_json(void *ctx,
>
> messages = notmuch_query_search_messages (query);
> message = notmuch_messages_get (messages);
> -if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> - &node) != NOTMUCH_STATUS_SUCCESS)
> +if (mime_node_open (ctx, message, crypto, &node) !=
> NOTMUCH_STATUS_SUCCESS)
> return 1;
>
> reply = create_reply_message (ctx, config, message, reply_all);
> diff --git a/notmuch-show.c b/notmuch-show.c
> index d254179..8b4d308 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -810,8 +810,7 @@ show_message (void *ctx,
> mime_node_t *root, *part;
> notmuch_status_t status;
>
> -status = mime_node_open (local, message, params->crypto.gpgctx,
> - params->crypto.decrypt, &root);
> +status = mime_node_open (local, message, &(params->crypto), &root);
The parenthesis around params->crypto are not needed, otherwise LGTM.
> if (status)
> goto DONE;
> part = mime_node_seek_dfs (root, (params->part < 0 ? 0 : params->part));
> --
> 1.7.10
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/6] cli: modify mime_node_context to use the new notmuch_crypto_t
On Thu, 17 May 2012, Jameson Graef Rollins wrote:
> This simplifies some more interfaces and gets rid of another #ifdef.
LGTM.
> ---
> mime-node.c | 23 +--
> 1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/mime-node.c b/mime-node.c
> index a95bdab..79a3654 100644
> --- a/mime-node.c
> +++ b/mime-node.c
> @@ -33,12 +33,7 @@ typedef struct mime_node_context {
> GMimeMessage *mime_message;
>
> /* Context provided by the caller. */
> -#ifdef GMIME_ATLEAST_26
> -GMimeCryptoContext *cryptoctx;
> -#else
> -GMimeCipherContext *cryptoctx;
> -#endif
> -notmuch_bool_t decrypt;
> +notmuch_crypto_t crypto;
> } mime_node_context_t;
>
> static int
> @@ -118,8 +113,8 @@ mime_node_open (const void *ctx, notmuch_message_t
> *message,
> goto DONE;
> }
>
> -mctx->cryptoctx = cryptoctx;
> -mctx->decrypt = decrypt;
> +mctx->crypto.gpgctx = cryptoctx;
> +mctx->crypto.decrypt = decrypt;
>
> /* Create the root node */
> root->part = GMIME_OBJECT (mctx->mime_message);
> @@ -195,7 +190,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
>
> /* Handle PGP/MIME parts */
> if (GMIME_IS_MULTIPART_ENCRYPTED (part)
> - && node->ctx->cryptoctx && node->ctx->decrypt) {
> + && node->ctx->crypto.gpgctx && node->ctx->crypto.decrypt) {
> if (node->nchildren != 2) {
> /* this violates RFC 3156 section 4, so we won't bother with it. */
> fprintf (stderr, "Error: %d part(s) for a multipart/encrypted "
> @@ -208,10 +203,10 @@ _mime_node_create (mime_node_t *parent, GMimeObject
> *part)
> #ifdef GMIME_ATLEAST_26
> GMimeDecryptResult *decrypt_result = NULL;
> node->decrypted_child = g_mime_multipart_encrypted_decrypt
> - (encrypteddata, node->ctx->cryptoctx, &decrypt_result, &err);
> + (encrypteddata, node->ctx->crypto.gpgctx, &decrypt_result,
> &err);
> #else
> node->decrypted_child = g_mime_multipart_encrypted_decrypt
> - (encrypteddata, node->ctx->cryptoctx, &err);
> + (encrypteddata, node->ctx->crypto.gpgctx, &err);
> #endif
> if (node->decrypted_child) {
> node->decrypt_success = node->verify_attempted = TRUE;
> @@ -229,7 +224,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
>(err ? err->message : "no error explanation given"));
> }
> }
> -} else if (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->cryptoctx) {
> +} else if (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto.gpgctx)
> {
> if (node->nchildren != 2) {
> /* this violates RFC 3156 section 5, so we won't bother with it. */
> fprintf (stderr, "Error: %d part(s) for a multipart/signed message "
> @@ -238,7 +233,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
> } else {
> #ifdef GMIME_ATLEAST_26
> node->sig_list = g_mime_multipart_signed_verify
> - (GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);
> + (GMIME_MULTIPART_SIGNED (part), node->ctx->crypto.gpgctx, &err);
> node->verify_attempted = TRUE;
>
> if (!node->sig_list)
> @@ -254,7 +249,7 @@ _mime_node_create (mime_node_t *parent, GMimeObject *part)
>* In GMime 2.6, they're both non-const, so we'll be able
>* to clean up this asymmetry. */
> GMimeSignatureValidity *sig_validity =
> g_mime_multipart_signed_verify
> - (GMIME_MULTIPART_SIGNED (part), node->ctx->cryptoctx, &err);
> + (GMIME_MULTIPART_SIGNED (part), node->ctx->crypto.gpgctx, &err);
> node->verify_attempted = TRUE;
> node->sig_validity = sig_validity;
> if (sig_validity) {
> --
> 1.7.10
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/6] cli: new crypto structure to store crypto contexts and parameters
On Thu, 17 May 2012, Jameson Graef Rollins wrote:
> The main point here is to keep track of the crypto stuff together in
> one place. In notmuch-show the crypto struct is a sub structure of
> the parameters struct. In notmuch-reply, which had been using a
> notmuch_show_params_t to store the crypto parameters, we can now just
> use the general crypto struct.
Looks good. My only (potentially unwarranted) worry is dropping the use
of notmuch_show_params_t in reply. This diverges the show/reply code,
making any future unification of them slightly harder. And if reply ever
needs params, we'll need to bring it back. But perhaps I worry too
much. :)
BR,
Jani.
>
> I slip in a name change of the crypto context itself to better reflect
> what the context is specifically for: it's actually a GPG context,
> which is a sub type of Crypto context. There are other types of
> Crypto contexts (Pkcs7 in particular) so we want to be clear.
>
> The following patches will use this to simplify some function
> interfaces.
> ---
> notmuch-client.h | 16 ++--
> notmuch-reply.c | 34 +-
> notmuch-show.c | 22 +++---
> 3 files changed, 38 insertions(+), 34 deletions(-)
>
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 19b7f01..2ad24cf 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -74,17 +74,21 @@ typedef struct notmuch_show_format {
> const char *message_set_end;
> } notmuch_show_format_t;
>
> +typedef struct notmuch_crypto {
> +#ifdef GMIME_ATLEAST_26
> +GMimeCryptoContext* gpgctx;
> +#else
> +GMimeCipherContext* gpgctx;
> +#endif
> +notmuch_bool_t decrypt;
> +} notmuch_crypto_t;
> +
> typedef struct notmuch_show_params {
> notmuch_bool_t entire_thread;
> notmuch_bool_t omit_excluded;
> notmuch_bool_t raw;
> int part;
> -#ifdef GMIME_ATLEAST_26
> -GMimeCryptoContext* cryptoctx;
> -#else
> -GMimeCipherContext* cryptoctx;
> -#endif
> -notmuch_bool_t decrypt;
> +notmuch_crypto_t crypto;
> } notmuch_show_params_t;
>
> /* There's no point in continuing when we've detected that we've done
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 7184a5d..ed87899 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -515,7 +515,7 @@ static int
> notmuch_reply_format_default(void *ctx,
>notmuch_config_t *config,
>notmuch_query_t *query,
> - notmuch_show_params_t *params,
> + notmuch_crypto_t *crypto,
>notmuch_bool_t reply_all)
> {
> GMimeMessage *reply;
> @@ -544,7 +544,7 @@ notmuch_reply_format_default(void *ctx,
> g_object_unref (G_OBJECT (reply));
> reply = NULL;
>
> - if (mime_node_open (ctx, message, params->cryptoctx, params->decrypt,
> + if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> &root) == NOTMUCH_STATUS_SUCCESS) {
> format_part_reply (root);
> talloc_free (root);
> @@ -559,7 +559,7 @@ static int
> notmuch_reply_format_json(void *ctx,
> notmuch_config_t *config,
> notmuch_query_t *query,
> - notmuch_show_params_t *params,
> + notmuch_crypto_t *crypto,
> notmuch_bool_t reply_all)
> {
> GMimeMessage *reply;
> @@ -574,7 +574,7 @@ notmuch_reply_format_json(void *ctx,
>
> messages = notmuch_query_search_messages (query);
> message = notmuch_messages_get (messages);
> -if (mime_node_open (ctx, message, params->cryptoctx, params->decrypt,
> +if (mime_node_open (ctx, message, crypto->gpgctx, crypto->decrypt,
> &node) != NOTMUCH_STATUS_SUCCESS)
> return 1;
>
> @@ -605,7 +605,7 @@ static int
> notmuch_reply_format_headers_only(void *ctx,
> notmuch_config_t *config,
> notmuch_query_t *query,
> - unused (notmuch_show_params_t *params),
> + unused (notmuch_crypto_t *crypto),
> notmuch_bool_t reply_all)
> {
> GMimeMessage *reply;
> @@ -674,8 +674,8 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
> notmuch_query_t *query;
> char *query_string;
> int opt_index, ret = 0;
> -int (*reply_format_func)(void *ctx, notmuch_config_t *config,
> notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t
> reply_all);
> -notmuch_show_params_t params = { .part = -1 };
> +int (*reply_format_func)(void *ctx, notmuch_config_t *config,
> notmuch_query_t *query, notmuch_crypto_t *crypto, notmuch_bool_t reply_all);
> +notmuch_crypto_t crypto = { .decrypt = FALSE };
> int format = FORMAT_DEFAULT;
> int reply_all = TRUE;
>
> @@ -689,7 +689,7 @@ notmuch_reply_command (voi
