[PATCH 4/5] cli/dump: define GZPUTS and use it in notmuch-dump
Similarly to GZPRINTF, this is a drop in replacement that can be
improved where needd.
---
notmuch-client.h | 1 +
notmuch-dump.c | 10 +-
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/notmuch-client.h b/notmuch-client.h
index 55d4d526..7efcb06f 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -499,6 +499,7 @@ print_status_gzbytes (const char *loc,
#define ASSERT_GZBYTES(file, bytes) ( (print_status_gzbytes(__location__,
file, bytes)) ? exit(1) : 0 )
#define GZPRINTF(file, fmt, ...) ASSERT_GZBYTES(file, gzprintf (file, fmt,
##__VA_ARGS__));
+#define GZPUTS(file, str) ASSERT_GZBYTES(file, gzputs (file, str));
#include "command-line-arguments.h"
diff --git a/notmuch-dump.c b/notmuch-dump.c
index ca45d885..fb322237 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -76,7 +76,7 @@ print_dump_header (gzFile output, int output_format, int
include)
NOTMUCH_DUMP_VERSION);
if (include & DUMP_INCLUDE_CONFIG) {
- gzputs (output, "config");
+ GZPUTS (output, "config");
sep = ",";
}
if (include & DUMP_INCLUDE_PROPERTIES) {
@@ -86,7 +86,7 @@ print_dump_header (gzFile output, int output_format, int
include)
if (include & DUMP_INCLUDE_TAGS) {
GZPRINTF (output, "%stags", sep);
}
-gzputs (output, "\n");
+GZPUTS (output, "\n");
}
static int
@@ -174,12 +174,12 @@ dump_tags_message (void *ctx,
const char *tag_str = notmuch_tags_get (tags);
if (! first)
- gzputs (output, " ");
+ GZPUTS (output, " ");
first = 0;
if (output_format == DUMP_FORMAT_SUP) {
- gzputs (output, tag_str);
+ GZPUTS (output, tag_str);
} else {
if (hex_encode (ctx, tag_str,
buffer_p, size_p) != HEX_SUCCESS) {
@@ -192,7 +192,7 @@ dump_tags_message (void *ctx,
}
if (output_format == DUMP_FORMAT_SUP) {
- gzputs (output, ")\n");
+ GZPUTS (output, ")\n");
} else {
if (make_boolean_term (ctx, "id", message_id,
buffer_p, size_p)) {
--
2.25.1
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 5/5] cli/dump: replace use of gzprintf with gzputs for config values
These can be large, and hit buffer limitations of gzprintf. --- notmuch-dump.c| 4 +++- test/T240-dump-restore.sh | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/notmuch-dump.c b/notmuch-dump.c index fb322237..23d7d20a 100644 --- a/notmuch-dump.c +++ b/notmuch-dump.c @@ -51,7 +51,9 @@ database_dump_config (notmuch_database_t *notmuch, gzFile output) goto DONE; } - GZPRINTF (output, " %s\n", buffer); + GZPUTS (output, " "); + GZPUTS (output, buffer); + GZPUTS (output, "\n"); } ret = EXIT_SUCCESS; diff --git a/test/T240-dump-restore.sh b/test/T240-dump-restore.sh index 374db5c2..3b6ab8fd 100755 --- a/test/T240-dump-restore.sh +++ b/test/T240-dump-restore.sh @@ -323,7 +323,6 @@ EOF test_expect_equal_file EXPECTED OUTPUT test_begin_subtest 'dumping large queries' -test_subtest_known_broken # This value repeat was found experimentally by binary search. The # config value after URL encoding is exactly 4096 bytes, which # suggests a buffer size bug. -- 2.25.1 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[no subject]
Here's a not too ambitious attempt to clean up the error handling on zlib output. It's bit gross to treat any error reported by zlib as fatal, but it's a step up from ignoring them, and it's in the client code, not in the library. The first patch splits out the first fix of id:20200410173039.yextmxk4wtgpl...@siegel.lan The last patch is a fix for the bug reported by qsx. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/5] util/zlib-extra.c: don't pass NULL to gzerror.
Although (as of 1.2.11) zlib checks this parameter before writing to
it, the docs don't promise to keep doing so, so be safe.
---
util/zlib-extra.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/util/zlib-extra.c b/util/zlib-extra.c
index f691cccf..e17a9731 100644
--- a/util/zlib-extra.c
+++ b/util/zlib-extra.c
@@ -79,8 +79,10 @@ 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 dummy;
+
if (status == UTIL_GZERROR)
- return gzerror (file, NULL);
+ return gzerror (file, &dummy);
else
return util_error_string (status);
}
--
2.25.1
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/5] cli/dump: define GZPRINTF macro and use it in place of gzprintf
This will at least catch errors, and can be replaced with more
sophisticated error handling where appropriate.
---
notmuch-client.h | 3 +++
notmuch-dump.c | 24
2 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/notmuch-client.h b/notmuch-client.h
index 467e1d84..55d4d526 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -497,6 +497,9 @@ print_status_gzbytes (const char *loc,
gzFile file,
int bytes);
+#define ASSERT_GZBYTES(file, bytes) ( (print_status_gzbytes(__location__,
file, bytes)) ? exit(1) : 0 )
+#define GZPRINTF(file, fmt, ...) ASSERT_GZBYTES(file, gzprintf (file, fmt,
##__VA_ARGS__));
+
#include "command-line-arguments.h"
extern const char *notmuch_requested_db_uuid;
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 65e02639..ca45d885 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -42,7 +42,7 @@ database_dump_config (notmuch_database_t *notmuch, gzFile
output)
notmuch_config_list_key (list));
goto DONE;
}
- gzprintf (output, "#@ %s", buffer);
+ GZPRINTF (output, "#@ %s", buffer);
if (hex_encode (notmuch, notmuch_config_list_value (list),
&buffer, &buffer_size) != HEX_SUCCESS) {
@@ -51,7 +51,7 @@ database_dump_config (notmuch_database_t *notmuch, gzFile
output)
goto DONE;
}
- gzprintf (output, " %s\n", buffer);
+ GZPRINTF (output, " %s\n", buffer);
}
ret = EXIT_SUCCESS;
@@ -71,7 +71,7 @@ print_dump_header (gzFile output, int output_format, int
include)
{
const char *sep = "";
-gzprintf (output, "#notmuch-dump %s:%d ",
+GZPRINTF (output, "#notmuch-dump %s:%d ",
(output_format == DUMP_FORMAT_SUP) ? "sup" : "batch-tag",
NOTMUCH_DUMP_VERSION);
@@ -80,11 +80,11 @@ print_dump_header (gzFile output, int output_format, int
include)
sep = ",";
}
if (include & DUMP_INCLUDE_PROPERTIES) {
- gzprintf (output, "%sproperties", sep);
+ GZPRINTF (output, "%sproperties", sep);
sep = ",";
}
if (include & DUMP_INCLUDE_TAGS) {
- gzprintf (output, "%stags", sep);
+ GZPRINTF (output, "%stags", sep);
}
gzputs (output, "\n");
}
@@ -115,7 +115,7 @@ dump_properties_message (void *ctx,
fprintf (stderr, "Error: failed to hex-encode message-id %s\n",
message_id);
return 1;
}
- gzprintf (output, "#= %s", *buffer_p);
+ GZPRINTF (output, "#= %s", *buffer_p);
first = false;
}
@@ -126,18 +126,18 @@ dump_properties_message (void *ctx,
fprintf (stderr, "Error: failed to hex-encode key %s\n", key);
return 1;
}
- gzprintf (output, " %s", *buffer_p);
+ GZPRINTF (output, " %s", *buffer_p);
if (hex_encode (ctx, val, buffer_p, size_p) != HEX_SUCCESS) {
fprintf (stderr, "Error: failed to hex-encode value %s\n", val);
return 1;
}
- gzprintf (output, "=%s", *buffer_p);
+ GZPRINTF (output, "=%s", *buffer_p);
}
notmuch_message_properties_destroy (list);
if (! first)
- gzprintf (output, "\n", *buffer_p);
+ GZPRINTF (output, "\n", *buffer_p);
return 0;
}
@@ -165,7 +165,7 @@ dump_tags_message (void *ctx,
}
if (output_format == DUMP_FORMAT_SUP) {
- gzprintf (output, "%s (", message_id);
+ GZPRINTF (output, "%s (", message_id);
}
for (notmuch_tags_t *tags = notmuch_message_get_tags (message);
@@ -187,7 +187,7 @@ dump_tags_message (void *ctx,
tag_str);
return EXIT_FAILURE;
}
- gzprintf (output, "+%s", *buffer_p);
+ GZPRINTF (output, "+%s", *buffer_p);
}
}
@@ -200,7 +200,7 @@ dump_tags_message (void *ctx,
message_id, strerror (errno));
return EXIT_FAILURE;
}
- gzprintf (output, " -- %s\n", *buffer_p);
+ GZPRINTF (output, " -- %s\n", *buffer_p);
}
return EXIT_SUCCESS;
}
--
2.25.1
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/5] status: add print_status_gzbytes
This is in the client code, rather than libnotmuch_util, because it
prints to stderr. Also it in pretends to generate notmuch status
codes.
---
notmuch-client.h | 8 +++-
status.c | 14 ++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/notmuch-client.h b/notmuch-client.h
index 89e15ba6..467e1d84 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -49,6 +49,7 @@
#include
#include
#include
+#include
#include "talloc-extra.h"
#include "crypto.h"
@@ -469,7 +470,7 @@ notmuch_database_dump (notmuch_database_t *notmuch,
dump_include_t include,
bool gzip_output);
-/* If status is non-zero (i.e. error) print appropriate
+/* If status indicates error print appropriate
* messages to stderr.
*/
@@ -491,6 +492,11 @@ print_status_database (const char *loc,
int
status_to_exit (notmuch_status_t status);
+notmuch_status_t
+print_status_gzbytes (const char *loc,
+ gzFile file,
+ int bytes);
+
#include "command-line-arguments.h"
extern const char *notmuch_requested_db_uuid;
diff --git a/status.c b/status.c
index d0ae47f4..09d82a17 100644
--- a/status.c
+++ b/status.c
@@ -72,3 +72,17 @@ status_to_exit (notmuch_status_t status)
return EXIT_FAILURE;
}
}
+
+notmuch_status_t
+print_status_gzbytes (const char *loc, gzFile file, int bytes)
+{
+if (bytes <= 0) {
+ int errnum;
+ const char *errstr = gzerror (file, &errnum);
+ fprintf (stderr, "%s: zlib error %s (%d)\n", loc, errstr, errnum);
+ return NOTMUCH_STATUS_FILE_ERROR;
+} else {
+ return NOTMUCH_STATUS_SUCCESS;
+}
+}
+
--
2.25.1
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] test: add known_broken test for dumping large stored queries
David Bremner writes: > 'qsx' reported a bug on #notmuch with notmuch-dump and large stored > queries. This test will pass (on my machine) if the value of `repeat' > is made smaller. I believe the underlying problem is explained by this comment in zlib.h The number of uncompressed bytes written is limited to 8191, or one less than the buffer size given to gzbuffer(). The caller should assure that this limit is not exceeded. If it is exceeded, then gzprintf() will return an error (0) with nothing written. So calling gzbuffer can increase this limit, but you still have to guess before writing for the first time. One way of interpreting this is that we should avoid using gzprintf where we cannot predict the output size. d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] test: add known_broken test for dumping large stored queries
'qsx' reported a bug on #notmuch with notmuch-dump and large stored
queries. This test will pass (on my machine) if the value of `repeat'
is made smaller.
---
test/T240-dump-restore.sh | 13 +
1 file changed, 13 insertions(+)
diff --git a/test/T240-dump-restore.sh b/test/T240-dump-restore.sh
index 0870ff92..374db5c2 100755
--- a/test/T240-dump-restore.sh
+++ b/test/T240-dump-restore.sh
@@ -322,6 +322,19 @@ EOF
test_expect_equal_file EXPECTED OUTPUT
+test_begin_subtest 'dumping large queries'
+test_subtest_known_broken
+# This value repeat was found experimentally by binary search. The
+# config value after URL encoding is exactly 4096 bytes, which
+# suggests a buffer size bug.
+repeat=1329
+notmuch config set query.big "$(seq -s' ' $repeat)"
+notmuch dump --include=config > OUTPUT
+notmuch config set query.big ''
+printf "#notmuch-dump batch-tag:3 config\n#@ query.big " > EXPECTED
+seq -s'%20' $repeat >> EXPECTED
+test_expect_equal_file EXPECTED OUTPUT
+
test_begin_subtest 'roundtripping random message-ids and tags'
${TEST_DIRECTORY}/random-corpus --config-path=${NOTMUCH_CONFIG} \
--
2.25.1
___
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
&errnum.
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,
&errnum));
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, &errnum));
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, ¬much))
@@ -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, &zlib_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, &errnum);
else
return util_error_string (status);
}
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
