We (finally) implement the XXX comment. It requires a bit of care not
to reparent all of the possible toplevel messages.
_notmuch_messages_has_next is not ready to be a public function yet,
since it punts on the mset case. We know in the one case it is called,
the notmuch_messages_t is just a regular list / iterator.
This is mainly to lay out the structure of the final code. The problem
isn't really solved yet, although some very simple cases are
better (hence the fixed test). We need two passes through the messages
because we need to be careful not to re-parent too many messages and
end up without any toplevel messages.
There is no public notmuch_message_list_t public interface, so to this
is added to the private API. We use it immediately in thread.cc;
future commits will use it further.
For non-root messages, this should not should anything currently, as
the messages are already added in date order. In the future we will
add some non-root messages in a second pass out of order and the
sorting will be useful. It does fix the order of multiple
root-messages (although it is overkill for that).
The current behaviour is at best under-documented. The modified test in
T470-missing-headers.sh previously relied on printf doing the right
thing with NULL, which seems icky.
The use of talloc_strdup here is probably overkill, but it avoids
having to enforce that thread->authors is never mutated outside
_resolve_thread_authors_string.
C99 stdbool turned 18 this year. There really is no reason to use our
own, except in the library interface for backward
compatibility. Convert the lib internally to stdbool.
Commit d5523ead90 ("Mark some structures in the library interface
with visibility=default attribute.") fixed some mixed visibility
issues with structs. With the symbol default visibility reversed, this
is no longer a problem.
Many of the external links found in the notmuch source can be resolved
using https instead of http. This changeset addresses as many as i
could find, without touching the e-mail corpus or expected outputs
found in tests.
_thread_set_subject_from_message sometimes replaces the subject, making the
cur_subject point to free'd memory
==6550== ERROR: AddressSanitizer: heap-use-after-free on address 0x601a0000bec0 at pc 0x4464a4 bp 0x7fffa40be910 sp 0x7fffa40be908
READ of size 1 at 0x601a0000bec0 thread T0
#0 0x4464a3 in _thread_add_matched_message /home/todd/.apps/notmuch/lib/thread.cc:369
#1 0x443c2c in notmuch_threads_get /home/todd/.apps/notmuch/lib/query.cc:496
#2 0x41d947 in do_search_threads /home/todd/.apps/notmuch/notmuch-search.c:131
#3 0x40a3fe in main /home/todd/.apps/notmuch/notmuch.c:345
#4 0x7f4e535b4ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
#5 0x40abe6 in _start ??:?
0x601a0000bec0 is located 96 bytes inside of 134-byte region [0x601a0000be60,0x601a0000bee6)
freed by thread T0 here:
#0 0x7f4e54e6933a in __interceptor_free ??:?
#1 0x7f4e54482fab in _talloc_free ??:?
previously allocated by thread T0 here:
#0 0x7f4e54e6941a in malloc ??:?
#1 0x7f4e54485b5d in talloc_strdup ??:?
==22884== ERROR: AddressSanitizer: heap-buffer-overflow on address 0x601600008291 at pc 0x7ff6295680e5 bp 0x7fff4ab9aa40 sp 0x7fff4ab9aa08
READ of size 1 at 0x601600008291 thread T0
#0 0x7ff6295680e4 in __interceptor_strcmp ??:?
#1 0x44763b in _thread_add_message /home/todd/.apps/notmuch/lib/thread.cc:255
#2 0x4459e8 in notmuch_threads_get /home/todd/.apps/notmuch/lib/query.cc:496
#3 0x41e2a7 in do_search_threads /home/todd/.apps/notmuch/notmuch-search.c:131
#4 0x40a408 in main /home/todd/.apps/notmuch/notmuch.c:345
#5 0x7ff627cb9ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
#6 0x40abf3 in _start ??:?
0x601600008291 is located 0 bytes to the right of 97-byte region [0x601600008230,0x601600008291)
allocated by thread T0 here:
#0 0x7ff62956e41a in malloc ??:?
#1 0x7ff628b8ab5d in talloc_strdup ??:?
Currently the thread is named based on either the oldest or newest
matching message (depending on the search order). If this message has
an empty subject, though, the thread will show up with an empty
subject in the search results. (See the thread starting with
`id:1412371140-21051-1-git-send-email-david@tethera.net` for an
example.)
This changes the behavior so it will use a non-empty name for the
thread if possible. We name threads based on (a) non-empty matches for
the query, and (b) the search order. If the search order is
oldest-first (as in the default inbox) it chooses the oldest matching
non-empty message as the subject. If the search order is newest-first
it chooses the newest one.
Currently, if a From-header is of the form:
"" <address@example.com>
the empty string will be treated as a valid real-name, and the entry
in the search results will be empty.
The new behavior here is that we treat an empty real-name field as if
it were null, so that the email address will be used in the search
results instead.
Signed-off-by: Jesse Rosenthal <jrosenthal@jhu.edu>
As noted in devel/STYLE, every private library function should start
with _notmuch. This patch corrects function naming that did not adhere
to this style in lib/notmuch-private.h. In particular, the old function
names that now begin with _notmuch are
notmuch_sha1_of_file
notmuch_sha1_of_string
notmuch_message_file_close
notmuch_message_file_get_header
notmuch_message_file_open
notmuch_message_get_author
notmuch_message_set_author
Signed-off-by: Charles Celerier <cceleri@cs.stanford.edu>
With some combination of clang and talloc, not using the return value
of talloc_steal() produces a warning. Ignore it, as talloc_steal() has
no failure modes per documentation.
Add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t so that it can
cover all four values of search --exclude in the cli.
Previously the way to avoid any message being marked excluded was to
pass in an empty list of excluded tags: since we now have an explicit
option we might as well honour it.
The enum is in a slightly strange order as the existing FALSE/TRUE
options correspond to the new
NOTMUCH_EXCLUDE_FLAG/NOTMUCH_EXCLUDE_TRUE options so this means we do
not need to bump the version number.
Indeed, an example of this is that the cli count and show still use
FALSE/TRUE and still work.
Notmuch automatically re-orders names of the format "Last, First" to
"First Last" when the associated email address is
First.Last@example.com. But, if a name is of the format "Last,First"
then notmuch will format the name as "irst Last". Handle any number of
spaces after the comma, including none.
Previously, getting the list of all messages in a thread required
recursively traversing the thread's message hierarchy, which was both
difficult and resulted in messages being out of order. This adds a
public function to retrieve an iterator over all of the messages in a
thread in oldest-first order.
Previously, thread.cc built up a list of all messages, then
proceeded to tear it apart to transform it into a list of
top-level messages. Now we simply build a new list of top-level
messages.
This simplifies the interface to _notmuch_message_add_reply,
eliminates the pointer acrobatics from
_resolve_thread_relationships, and will enable us to do things
with the list of all messages in the following patches.
Previously, there were various opportunities for memory leaks in the
error-handling paths of this function. Use a local talloc context and
some reparenting to make eliminate these leaks, while keeping the
control flow simple.
Add the NOTMUCH_MESSAGE_FLAG_EXCLUDED flag to
notmuch_query_search_threads. Implemented by inspecting the tags
directly in _notmuch_thread_create/_thread_add_message rather than as
a Xapian query for speed reasons.
Note notmuch_thread_get_matched_messages now returns the number of
non-excluded matching messages. This API is not totally desirable but
fixing it means breaking binary compatibility so we delay that.
As of gcc 4.6, there are new warnings from -Wattributes along the lines of:
warning: ‘_notmuch_messages’ declared with greater visibility
than the type of its field ‘_notmuch_messages::iterator’
[-Wattributes]
To squelch these, we decorate all such containing structs with
__attribute__((visibility("default"))). We take care to let only the
C++ compiler see this, (since the C compiler would otherwise warn
about ignored visibility attributes on types).
This replaces the guts of the filename list and tag list, making those
interfaces simple iterators over the generic string list. The
directory, message filename, and tags-related code now build generic
string lists and then wraps them in specific iterators. The real wins
come in later patches, when we use these for even more generic
functionality.
As a nice side-effect, this also eliminates the annoying dependency on
GList in the tag list.
With talloc, we were already freeing all memory by the time we exited
the loop, but that didn't help with excess use of memory inside the
loop, (which was mostly from tallocing some objects with the incorrect
parent).
Thanks to Andrew Tridgell for sitting next to me and teaching me to
use talloc_report_full to find these leaks.
This reduces thread search's 1+2t Xapian queries (where t is the
number of matched threads) to 1+t queries and constructs exactly one
notmuch_message_t for each message instead of 2 to 3.
notmuch_query_search_threads eagerly fetches the docids of all
messages matching the user query instead of lazily constructing
message objects and fetching thread ID's from term lists.
_notmuch_thread_create takes a seed docid and the set of all matched
docids and uses a single Xapian query to expand this docid to its
containing thread, using the matched docid set to determine which
messages in the thread match the user query instead of using a second
Xapian query.
This reduces the amount of time required to load my inbox from 4.523
seconds to 3.025 seconds (1.5X faster).
We really want to change the thread subject at the same time we set
the date, (if the sort order indicates this is necessary). The
previous code for setting the thread subject was sensitive on the
query sort when adding matching messages. An independent bug fix is
about to change that query sort order, so we remove the dependency on
it here.
These various functions and data are all used only locally, so should
be marked static. Ensuring we get these right will avoid us accidentally
leaking unintended symbols through the library interface.
Admittedly, an author name ending in ',' guarantees this is spam, and
indeed this was triggered by a spam email, but that doesn't mean we
shouldn't handle this case correctly.
We now check that there is actually a component of the name (presumably
the first name) after the comma in the author name.
Signed-off-by: Dirk Hohndel <hohndel@infradead.org>
Just before releasing 0.3 we received reports of crashes that were
bisected to the commit adding thread-author moving. Sure enough,
valgrind pointed to buffer overruns in _thread_move_matched_author.
Rather than trying to make sense of all the by strncpy, strchr, +1,
and +2 of that code, I reimplemented thread-author ordering with a
pair of hash tables and an array.
Valgrind is at least happy now on the test cases it was complaining
about previously.
This patch only addresses the typical Outlook/Exchange case
where we have "Last, First" <first.last@company.com> or
"Last, First MI" <first.mi.last@company.com>.
In the future we should be more fexible as to the formats
we recognize, but for now we address this one as it is the
Exchange default setting and therefore the most common one.
Signed-off-by: Dirk Hohndel <hohndel@infradead.org>
When displaying threads as result of a search it makes sense to list those
authors first who match the search. The matching authors are separated from the
non-matching ones with a '|' instead of a ','
Imagine the default "+inbox" query. Those mails in the thread that
match the query are actually "new" (whatever that means). And some
people seem to think that it would be much better to see those author
names first. For example, imagine a long and drawn out thread that once
was started by me; you have long read the older part of the thread and
removed the inbox tag. Whenever a new email comes in on this thread,
prior to this patch the author column in the search display will first show
"Dirk Hohndel" - I think it should first show the actual author(s) of the new
mail(s).
Signed-off-by: Dirk Hohndel <hohndel@infradead.org>
Sebastian Spaeth reported [*] a segfault within libnotmuch when
running notmuch operations while an asyncronous offlineimap job had
removed some files from the mail store. Avoid this by handling all
cases where notmuch_message_get_header could return NULL.
[*] See message id:87d3xqti3o.fsf@SSpaeth.de on notmuch@notmuchmail.org
The thread-naming feature depends on the matched messages being passed
down in a precise order, (the order of the top-level search). We fix
the feature by passing that sort order down.
