segfault in T590-libconfig notmuch_database_get_config_list: closed db
The test mentioned in the subject triggers a segfault. This is with
xapian-core-1.4.16 on OpenBSD. Here is the backtrace:
#0 0x05c9d491bec2 in Xapian::TermIterator::decref() () from
/usr/local/lib/libxapian.so.5.1
#1 0x05c948827062 in Xapian::TermIterator::~TermIterator
(this=0x5c9d2fdb068) at /usr/local/include/xapian/termiterator.h:85
#2 _notmuch_config_list_destroy (list=) at lib/config.cc:38
#3 0x05c97e08050a in ?? () from /usr/local/lib/libtalloc.so.1.1
#4 0x05c94882700f in notmuch_database_get_config_list
(notmuch=0x5c9cb714e60, prefix=, out=0x7f7c98e8) at
lib/config.cc:137
#5 0x05c6ff617c37 in main (argc=2, argv=0x7f7c9998) at test2.c:16
For convenience, here is test2.c:
<- begin test2.c ->
#include
#include
#include
int main (int argc, char** argv)
{
notmuch_database_t *db;
char *val;
notmuch_status_t stat;
EXPECT0(notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE,
));
{
notmuch_config_list_t *list;
EXPECT0(notmuch_database_close (db));
stat = notmuch_database_get_config_list (db, "nonexistent", );
printf("%d\n", stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
}
EXPECT0(notmuch_database_destroy(db));
}
<- end test2.c ->
In frame 4, we see that the iterator was not initialized:
(gdb) p *list
$7 = {notmuch = 0x5c9cb714e60, iterator = {internal = 0xdfdfdfdfdfdfdfdf},
current_key = 0x0, current_val = 0x0}
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
[PATCH] test: sort the output of the "prefix" test in T610-message-property
This test extracts values from a (key,value) map where multiple entries
can have the same key, and the entries are sorted by key, but not by
value. The test incorrectly assumes that the values will be sorted as
well, so sort the output.
---
test/T610-message-property.sh | 12
1 file changed, 12 insertions(+)
diff --git a/test/T610-message-property.sh b/test/T610-message-property.sh
index 53a0be3b..b8774230 100755
--- a/test/T610-message-property.sh
+++ b/test/T610-message-property.sh
@@ -186,6 +186,18 @@ EXPECT0(notmuch_message_add_property (message, "testkey3",
"testvalue3"));
EXPECT0(notmuch_message_add_property (message, "testkey3", "alice3"));
print_properties (message, "testkey", FALSE);
EOF
+# expected: 4 values for testkey1, 3 values for testkey3
+# they are not guaranteed to be sorted, so sort them, leaving the first
+# line '== stdout ==' and the end ('== stderr ==' and whatever error
+# may have been printed) alone
+mv OUTPUT unsorted_OUTPUT
+awk ' NR == 1 { print; next } \
+ NR < 6 { print | "sort"; next } \
+ NR == 6 { close("sort") } \
+ NR < 9 { print | "sort"; next } \
+ NR == 9 { close("sort") } \
+ { print }' unsorted_OUTPUT > OUTPUT
+rm unsorted_OUTPUT
cat <<'EOF' >EXPECTED
== stdout ==
alice
--
2.26.1
---
a better version of the previous patch I sent with head/tail replaced by an awk
script by Tomi Ollila which is clearer, and comments added in the test for
clarity.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] test: sort the output of the "prefix" test in T610-message-property as needed
the "prefix" test in T610-message-property extracts values from a (key,value) map where multiple entries can have the same key, and the entries are sorted by key, but not by value. The test incorrectly assumes that the values will be sorted as well, so correct this by splitting the output using head and tail and sorting each chunk using sort. NB: the relevant key/values are as follows. testkey1: alice, testvalue1, testvalue2, bob testkey3: alice3, bob3, testvalue3 --- test/T610-message-property.sh | 5 + 1 file changed, 5 insertions(+) diff --git a/test/T610-message-property.sh b/test/T610-message-property.sh index 53a0be3b..e1be2fc3 100755 --- a/test/T610-message-property.sh +++ b/test/T610-message-property.sh @@ -186,6 +186,11 @@ EXPECT0(notmuch_message_add_property (message, "testkey3", "testvalue3")); EXPECT0(notmuch_message_add_property (message, "testkey3", "alice3")); print_properties (message, "testkey", FALSE); EOF +mv OUTPUT unsorted_OUTPUT +head -n 5 unsorted_OUTPUT | sort >OUTPUT +tail -n +6 unsorted_OUTPUT | head -n 3 | sort >>OUTPUT +tail -n +9 unsorted_OUTPUT >>OUTPUT +rm unsorted_OUTPUT cat <<'EOF' >EXPECTED == stdout == alice -- 2.26.0 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] after gzgets(), Z_STREAM_END means EOF, not error
On Tue, Apr 14, 2020 at 05:38:32PM -0300, David Bremner wrote: > Olivier Taïbi writes: > > PS: out of curiosity, why bother with the --gzip feature in notmuch dump > > and restore when the user can simply pipe to/from a gzip/bzip2/xz/... > > command? > > I believe the original motivation (in 2014) was to make the dumping > process more atomic. I guess you could dig up the mailing list > discussion from the time if you were interested. I'd be reluctant to > remove the feature after 6 six years. Thanks for the explanation. > > PPS: apart from dump and restore (and the indirect use of xapian), it > > seems that the only other use of zlib in notmuch is in > > format_part_mbox() in notmuch-show.c, which is able to read a compressed > > email (it seems that dovecot has an option to write emails in maildir > > format in this way to save space).> Do I understand correctly that > > notmuch does not support indexing compressed email, and if so what is > > the point of using zlib in format_part_mbox()? > > No, that's not correct. Notmuch does support indexing gzip compressed > mail as of version 0.29. IIRC that part uses GMime streams to do the > decompression (probably also using zlib indirectly). Thanks, that's good to know. > It would be helpful to move the discussion not intended to be part of > the commit message after --- (see https://notmuchmail.org/contributing/ > for details). Sorry about that, I read the "bugs" section of the website but did not notice these guidelines. I'll do my best to do better next time. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] after gzgets(), Z_STREAM_END means EOF, not error
As suggested by David Bremner in
https://notmuchmail.org/pipermail/notmuch/2020/029288.html
here is the patch for bug #3: after gzgets() returns NULL (meaning EOF
or error), the error code Z_STREAM_END means EOF and not error.
Context: I am compiling notmuch on OpenBSD which has a rather old zlib
1.2.3. It seems that the behaviour of gzgets() changed slightly between
this version and more recent versions, but the manual does not reflect
that change. Note that zlib's manual:
- does not specify which error code (Z_OK or Z_STREAM_END) is set when
EOF is reached,
- does not indicate the meaning of Z_STREAM_END after gzgets(), but
based on its meaning as a possible return value of inflate(), I would
guess that it means EOF.
PS: out of curiosity, why bother with the --gzip feature in notmuch dump
and restore when the user can simply pipe to/from a gzip/bzip2/xz/...
command? It seems that this feature adds complication, some of this
complication being due to the fact that the behaviour of zlib is not as
well-defined as stdio. Moreover using pipes allows to check that the
compressed dump decompresses as it should, e.g.:
$ mkfifo pipe
$ dump.gz &
$ notmuch dump | tee pipe | cksum
$ wait && zcat dump.gz | cksum ; rm pipe
This could be simplified with process substitution for shells that have
this feature, and the checksum comparison can certainly be made
automatic in a backup script.
Disclosure: I am biased because I am currently patching notmuch-dump.c
to use stdio instead of zlib in order to port notmuch to OpenBSD, since
OpenBSD's older zlib does not have the "T" mode for gzopen(), so with
zlib the only choice would to compress the output. Perhaps zlib will be
updated in OpenBSD in the future, but this is a short-term solution that
seems to not be too much trouble to maintain downstream.
PPS: apart from dump and restore (and the indirect use of xapian), it
seems that the only other use of zlib in notmuch is in
format_part_mbox() in notmuch-show.c, which is able to read a compressed
email (it seems that dovecot has an option to write emails in maildir
format in this way to save space). Do I understand correctly that
notmuch does not support indexing compressed email, and if so what is
the point of using zlib in format_part_mbox()?
diff --git a/util/zlib-extra.c b/util/zlib-extra.c
index 623f6d62..2d2d2414 100644
--- a/util/zlib-extra.c
+++ b/util/zlib-extra.c
@@ -47,6 +47,7 @@ gz_getline (void *talloc_ctx, char **bufptr, ssize_t
*bytes_read, gzFile stream)
int zlib_status = 0;
(void) gzerror (stream, _status);
switch (zlib_status) {
+ case Z_STREAM_END:
case Z_OK:
/* no data read before EOF */
if (offset == 0)
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] gzerror() after gzclose_r() is a use after free
As suggested by David Bremner in
https://notmuchmail.org/pipermail/notmuch/2020/029288.html
here is a separate patch for bug #2: calling gzerror() (indirectly via
gzerror_str()) after gzclose_r is a use after free, according to zlib's manual.
diff --git a/notmuch-restore.c b/notmuch-restore.c
index 9a8b7fb5..e2dc3d45 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -237,6 +237,7 @@ notmuch_restore_command (notmuch_config_t *config, int
argc, char *argv[])
int opt_index;
int include = 0;
int input_format = DUMP_FORMAT_AUTO;
+int errnum;
if (notmuch_database_open (notmuch_config_get_database_path (config),
NOTMUCH_DATABASE_MODE_READ_WRITE, ))
@@ -448,10 +449,13 @@ notmuch_restore_command (notmuch_config_t *config, int
argc, char *argv[])
if (notmuch)
notmuch_database_destroy (notmuch);
-if (input && gzclose_r (input)) {
- fprintf (stderr, "Error closing %s: %s\n",
-name_for_error, gzerror_str (input));
- ret = EXIT_FAILURE;
+if (input) {
+ errnum = gzclose_r (input);
+ if (errnum) {
+ fprintf (stderr, "Error closing %s: %d\n",
+name_for_error, errnum);
+ ret = EXIT_FAILURE;
+ }
}
return ret ? EXIT_FAILURE : EXIT_SUCCESS;
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] zlib-related bugs
the following diff addresses 3 zlib-related bugs in notmuch.
1) the second argument of gzerror() cannot be NULL, so replace it by a dummy
2) gzerror() cannot be closed after gzclosed(), so just print the error value
instead.
3) in gz_getline(), if gz_error sets its second argument to Z_STREAM_END then
there was no error (only EOF). Unfortunately the zlib manual is not very
clear on the meaning of Z_STREAM_END, but I don't see how it could be an
error. I found this issue by using notmuch on OpenBSD, which has an old
zlib. I encountered other issues with notmuch on OpenBSD (e.g. there is no
transparency mode in this older zlib, so notmuch dump output is always
gzipped), but they do not seem to be bugs in notmuch.
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 65e02639..5d4b2fa7 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -287,6 +287,7 @@ notmuch_database_dump (notmuch_database_t *notmuch,
int outfd = -1;
int ret = -1;
+int errnum;
if (output_file_name) {
tempname = talloc_asprintf (notmuch, "%s.XX", output_file_name);
@@ -316,7 +317,7 @@ notmuch_database_dump (notmuch_database_t *notmuch,
ret = gzflush (output, Z_FINISH);
if (ret) {
- fprintf (stderr, "Error flushing output: %s\n", gzerror (output, NULL));
+ fprintf (stderr, "Error flushing output: %s\n", gzerror (output,
));
goto DONE;
}
@@ -332,7 +333,7 @@ notmuch_database_dump (notmuch_database_t *notmuch,
ret = gzclose_w (output);
if (ret) {
fprintf (stderr, "Error closing %s: %s\n", name_for_error,
-gzerror (output, NULL));
+gzerror (output, ));
ret = EXIT_FAILURE;
output = NULL;
goto DONE;
diff --git a/notmuch-restore.c b/notmuch-restore.c
index 4b509d95..e2dc3d45 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -237,6 +237,7 @@ notmuch_restore_command (notmuch_config_t *config, int
argc, char *argv[])
int opt_index;
int include = 0;
int input_format = DUMP_FORMAT_AUTO;
+int errnum;
if (notmuch_database_open (notmuch_config_get_database_path (config),
NOTMUCH_DATABASE_MODE_READ_WRITE, ))
@@ -448,10 +449,13 @@ notmuch_restore_command (notmuch_config_t *config, int
argc, char *argv[])
if (notmuch)
notmuch_database_destroy (notmuch);
-if (input && gzclose_r (input)) {
- fprintf (stderr, "Error closing %s: %s\n",
-name_for_error, gzerror (input, NULL));
- ret = EXIT_FAILURE;
+if (input) {
+ errnum = gzclose_r (input);
+ if (errnum) {
+ fprintf (stderr, "Error closing %s: %d\n",
+name_for_error, errnum);
+ ret = EXIT_FAILURE;
+ }
}
return ret ? EXIT_FAILURE : EXIT_SUCCESS;
diff --git a/util/zlib-extra.c b/util/zlib-extra.c
index f691cccf..3800fa60 100644
--- a/util/zlib-extra.c
+++ b/util/zlib-extra.c
@@ -47,6 +47,7 @@ gz_getline (void *talloc_ctx, char **bufptr, ssize_t
*bytes_read, gzFile stream)
int zlib_status = 0;
(void) gzerror (stream, _status);
switch (zlib_status) {
+ case Z_STREAM_END:
case Z_OK:
/* no data read before EOF */
if (offset == 0)
@@ -79,8 +80,9 @@ gz_getline (void *talloc_ctx, char **bufptr, ssize_t
*bytes_read, gzFile stream)
const char *
gz_error_string (util_status_t status, gzFile file)
{
+int errnum;
if (status == UTIL_GZERROR)
- return gzerror (file, NULL);
+ return gzerror (file, );
else
return util_error_string (status);
}
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
