Re: [PATCH v2] emacs: Add notmuch-update-search-tags
On 2017-08-26 10:23, Mark Walters wrote: 1) Suppose a new message arrives in the thread. Then triggering a tag change in the show buffer, (eg notmuch automatically removing an unread tag) pulls that message into original search buffer. 2) If that new message has some tags that aren't in the thread they will get marked as "added tags" Yes, AFAICT this is correct. I think 3 is probably rare enough not to be real concern (though we should make sure we don't actually give an error). But 1 and 2 seem undesirable. If the thread is gone, then it won't be in the results of the tag query, therefore it simply won't be updated. If we do that then I think the only change in these buffers is the tag updates, and exactly as if you changed those tags in the search buffer itself, the threads shown etc don't change. What do you think? I think it's doable in theory, just not efficiently. In order to fetch correct results for the messages currently displayed in the search buffer, we need to pass those exact message IDs to notmuch. If a tagging operation affects many messages from a large search buffer, doing one notmuch search invocation per thread can add up to a lot of notmuch invocations. It may be possible to optimize this down to one batch search query per notmuch-search buffer - however, this results in a large search query. Not only would one take a while to execute, but the resulting query can become too large to be passed as a command-line parameter, and "notmuch search" does not seem to have a --batch switch to read queries from standard input. Even if the above were to work sufficiently well, it is still necessary to do one notmuch invocation per search buffer, as individual search buffers may capture results from different points in time. In the patch above just one additional invocation in total is necessary. So, I don't see a way to do this in a sufficiently efficient way. Performance does matter here, as notmuch will perform tag updates in situations where a user shouldn't expect significant delays - e.g. holding down the "down arrow" key in a notmuch-show buffer with some unread messages will cause notmuch to remove the "unread" tag as point travels over each unread message. What do you think? -- Best regards, Vladimir ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 5/5] test/smtp-dummy: convert to 'goto DONE' style
Clean up several cppcheck warnings of the form
- test/smtp-dummy.c:170: error: Resource leak: output
Conform to overall notmuch code style.
---
test/smtp-dummy.c | 47 +++
1 file changed, 31 insertions(+), 16 deletions(-)
diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c
index fec2b8ad..71992edd 100644
--- a/test/smtp-dummy.c
+++ b/test/smtp-dummy.c
@@ -121,13 +121,14 @@ main (int argc, char *argv[])
{
const char *progname;
char *output_filename;
-FILE *peer_file, *output;
-int sock, peer, err;
+FILE *peer_file = NULL, *output = NULL;
+int sock = -1, peer, err;
struct sockaddr_in addr, peer_addr;
struct hostent *hostinfo;
socklen_t peer_addr_len;
int reuse;
int background;
+int ret = 0;
progname = argv[0];
@@ -160,14 +161,16 @@ main (int argc, char *argv[])
if (output == NULL) {
fprintf (stderr, "Failed to open %s for writing: %s\n",
output_filename, strerror (errno));
- return 1;
+ ret = 1;
+ goto DONE;
}
sock = socket (AF_INET, SOCK_STREAM, 0);
if (sock == -1) {
fprintf (stderr, "Error: socket() failed: %s\n",
strerror (errno));
- return 1;
+ ret = 1;
+ goto DONE;
}
reuse = 1;
@@ -175,13 +178,15 @@ main (int argc, char *argv[])
if (err) {
fprintf (stderr, "Error: setsockopt() failed: %s\n",
strerror (errno));
- return 1;
+ ret = 1;
+ goto DONE;
}
hostinfo = gethostbyname ("localhost");
if (hostinfo == NULL) {
fprintf (stderr, "Unknown host: localhost\n");
- return 1;
+ ret = 1;
+ goto DONE;
}
memset (&addr, 0, sizeof (addr));
@@ -193,7 +198,8 @@ main (int argc, char *argv[])
fprintf (stderr, "Error: bind() failed: %s\n",
strerror (errno));
close (sock);
- return 1;
+ ret = 1;
+ goto DONE;
}
err = listen (sock, 1);
@@ -201,7 +207,8 @@ main (int argc, char *argv[])
fprintf (stderr, "Error: listen() failed: %s\n",
strerror (errno));
close (sock);
- return 1;
+ ret = 1;
+ goto DONE;
}
if (background) {
@@ -210,13 +217,15 @@ main (int argc, char *argv[])
printf ("smtp_dummy_pid='%d'\n", pid);
fflush (stdout);
close (sock);
- return 0;
+ ret = 0;
+ goto DONE;
}
if (pid < 0) {
fprintf (stderr, "Error: fork() failed: %s\n",
strerror (errno));
close (sock);
- return 1;
+ ret = 1;
+ goto DONE;
}
/* Reached if pid == 0 (the child process). */
/* Close stdout so that the one interested in pid value will
@@ -239,21 +248,27 @@ main (int argc, char *argv[])
if (peer == -1) {
fprintf (stderr, "Error: accept() failed: %s\n",
strerror (errno));
- return 1;
+ ret = 1;
+ goto DONE;
}
peer_file = fdopen (peer, "w+");
if (peer_file == NULL) {
fprintf (stderr, "Error: fdopen() failed: %s\n",
strerror (errno));
- return 1;
+ ret = 1;
+ goto DONE;
}
do_smtp_to_file (peer_file, output);
-fclose (output);
-fclose (peer_file);
-close (sock);
+ DONE:
+if (output)
+ fclose (output);
+if (peer_file)
+ fclose (peer_file);
+if (sock >= 0)
+ close (sock);
-return 0;
+return ret;
}
--
2.14.1
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
make notmuch cppcheck clean
I don't know if cppcheck is the best static checker there is, but it's found a few problems already, and I had it installed, so this is an initial response to Jani's suggestion of having one or more targets to run static checkers. I punted on a few of the tricker issues: cppcheck is run serially here, even though it scales rather well, and the target is optional. The rest of the series fixes complaints of cppcheck so that in principle, we could run it automatically. My main reason for not doing so is that I don't want builds that break with a new release of cppcheck. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/5] cppcheck: close files during shutdown
Fix the following cppcheck errors:
notmuch-count.c:207: error: Resource leak: input
notmuch-tag.c:238: error: Resource leak: input
We know that the program is shutting down here, but it does no harm to
clean up a bit.
---
notmuch-count.c | 2 ++
notmuch-tag.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/notmuch-count.c b/notmuch-count.c
index 50b0c193..97281374 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -204,6 +204,8 @@ notmuch_count_command (notmuch_config_t *config, int argc,
char *argv[])
if (batch && opt_index != argc) {
fprintf (stderr, "--batch and query string are not compatible\n");
+ if (input)
+ fclose (input);
return EXIT_FAILURE;
}
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 9c03754d..130de634 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -235,6 +235,8 @@ notmuch_tag_command (notmuch_config_t *config, int argc,
char *argv[])
if (batch) {
if (opt_index != argc) {
fprintf (stderr, "Can't specify both cmdline and stdin!\n");
+ if (input)
+ fclose (input);
return EXIT_FAILURE;
}
} else {
--
2.14.1
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/5] cppcheck: call va_end in _internal_error
fix for: util/error_util.c:38: error: va_list 'va_args' was opened but not closed by va_end() This makes the code more copy-pastable, if nothing else --- util/error_util.c | 1 + 1 file changed, 1 insertion(+) diff --git a/util/error_util.c b/util/error_util.c index 778bbd52..e64162c7 100644 --- a/util/error_util.c +++ b/util/error_util.c @@ -34,6 +34,7 @@ _internal_error (const char *format, ...) fprintf (stderr, "Internal error: "); vfprintf (stderr, format, va_args); +va_end (va_args); exit (1); } -- 2.14.1 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/5] test/smtp-dummy: uncrustify
For some reason lost in the mists of time this code was indented 8 spaces.
---
test/smtp-dummy.c | 336 +++---
1 file changed, 168 insertions(+), 168 deletions(-)
diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c
index a629927c..fec2b8ad 100644
--- a/test/smtp-dummy.c
+++ b/test/smtp-dummy.c
@@ -49,211 +49,211 @@
static void
receive_data_to_file (FILE *peer, FILE *output)
{
- char *line = NULL;
- size_t line_size;
- ssize_t line_len;
+char *line = NULL;
+size_t line_size;
+ssize_t line_len;
- while ((line_len = getline (&line, &line_size, peer)) != -1) {
- if (STRNCMP_LITERAL (line, ".\r\n") == 0)
- break;
- if (line_len < 2)
- continue;
- if (line[line_len-1] == '\n' && line[line_len-2] == '\r') {
- line[line_len-2] = '\n';
- line[line_len-1] = '\0';
- }
- fprintf (output, "%s",
-line[0] == '.' ? line + 1 : line);
+while ((line_len = getline (&line, &line_size, peer)) != -1) {
+ if (STRNCMP_LITERAL (line, ".\r\n") == 0)
+ break;
+ if (line_len < 2)
+ continue;
+ if (line[line_len - 1] == '\n' && line[line_len - 2] == '\r') {
+ line[line_len - 2] = '\n';
+ line[line_len - 1] = '\0';
}
+ fprintf (output, "%s",
+line[0] == '.' ? line + 1 : line);
+}
- free (line);
+free (line);
}
static int
process_command (FILE *peer, FILE *output, const char *command)
{
- if (STRNCMP_LITERAL (command, "EHLO ") == 0) {
- fprintf (peer, "502 not implemented\r\n");
- fflush (peer);
- } else if (STRNCMP_LITERAL (command, "HELO ") == 0) {
- fprintf (peer, "250 localhost\r\n");
- fflush (peer);
- } else if (STRNCMP_LITERAL (command, "MAIL FROM:") == 0 ||
- STRNCMP_LITERAL (command, "RCPT TO:") == 0) {
- fprintf (peer, "250 OK\r\n");
- fflush (peer);
- } else if (STRNCMP_LITERAL (command, "DATA") == 0) {
- fprintf (peer, "354 End data with .\r\n");
- fflush (peer);
- receive_data_to_file (peer, output);
- fprintf (peer, "250 OK\r\n");
- fflush (peer);
- } else if (STRNCMP_LITERAL (command, "QUIT") == 0) {
- fprintf (peer, "221 BYE\r\n");
- fflush (peer);
- return 1;
- } else {
- fprintf (stderr, "Unknown command: %s\n", command);
- }
- return 0;
+if (STRNCMP_LITERAL (command, "EHLO ") == 0) {
+ fprintf (peer, "502 not implemented\r\n");
+ fflush (peer);
+} else if (STRNCMP_LITERAL (command, "HELO ") == 0) {
+ fprintf (peer, "250 localhost\r\n");
+ fflush (peer);
+} else if (STRNCMP_LITERAL (command, "MAIL FROM:") == 0 ||
+ STRNCMP_LITERAL (command, "RCPT TO:") == 0) {
+ fprintf (peer, "250 OK\r\n");
+ fflush (peer);
+} else if (STRNCMP_LITERAL (command, "DATA") == 0) {
+ fprintf (peer, "354 End data with .\r\n");
+ fflush (peer);
+ receive_data_to_file (peer, output);
+ fprintf (peer, "250 OK\r\n");
+ fflush (peer);
+} else if (STRNCMP_LITERAL (command, "QUIT") == 0) {
+ fprintf (peer, "221 BYE\r\n");
+ fflush (peer);
+ return 1;
+} else {
+ fprintf (stderr, "Unknown command: %s\n", command);
+}
+return 0;
}
static void
do_smtp_to_file (FILE *peer, FILE *output)
{
- char *line = NULL;
- size_t line_size;
- ssize_t line_len;
+char *line = NULL;
+size_t line_size;
+ssize_t line_len;
- fprintf (peer, "220 localhost smtp-dummy\r\n");
- fflush (peer);
+fprintf (peer, "220 localhost smtp-dummy\r\n");
+fflush (peer);
- while ((line_len = getline (&line, &line_size, peer)) != -1) {
- if (process_command (peer, output, line))
- break;
- }
+while ((line_len = getline (&line, &line_size, peer)) != -1) {
+ if (process_command (peer, output, line))
+ break;
+}
- free (line);
+free (line);
}
int
main (int argc, char *argv[])
{
- const char * progname;
- char *output_filename;
- FILE *peer_file, *output;
- int sock, peer, err;
- struct sockaddr_in addr, peer_addr;
- struct hostent *hostinfo;
- socklen_t peer_addr_len;
- int reuse;
- int background;
+const char *progname;
+char *output_filename;
+FILE *peer_file, *output;
+int sock, peer, err;
+struct sockaddr_in addr, peer_addr;
+struct hostent *hostinfo;
+socklen_t peer_addr_len;
+int reuse;
+int background;
- progname = argv[0];
+progname = argv[0];
- bac
[PATCH 1/5] build: add target to run cppcheck
The advantage of having a target as opposed to running cppcheck by
hand
- reuse list of source files
- output errors in a format parsable, e.g. by emacs
- returns exit code 1 on any error, for possibly use in other
targets.
Things not addressed here
- parallelism. Doing this correctly seems like a rabbit-hole.
- what target to invoke this from. The 8s delay (on my machine) seems
acceptable, but not necessarily the resulting fragility in
autobuilders.
---
Makefile.local | 10 ++
configure | 11 +++
2 files changed, 21 insertions(+)
diff --git a/Makefile.local b/Makefile.local
index af12ca7f..6a5168f1 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -283,6 +283,16 @@ CLEAN := $(CLEAN) version.stamp notmuch-*.tar.gz.tmp
DISTCLEAN := $(DISTCLEAN) .first-build-message Makefile.config sh.config
+CLEAN := $(CLEAN) cppcheck.stamp
+cppcheck: cppcheck.stamp
+cppcheck.stamp: $(SRCS)
+ifeq ($(HAVE_CPPCHECK),1)
+ cppcheck --template="{file}:{line}: {severity}: {message}" --quiet
--error-exitcode=1 ${SRCS}
+ touch $@
+else
+ @echo No cppcheck installed; skipping static checking
+endif
+
DEPS := $(SRCS:%.c=.deps/%.d)
DEPS := $(DEPS:%.cc=.deps/%.d)
-include $(DEPS)
diff --git a/configure b/configure
index c5e2ffed..364854f3 100755
--- a/configure
+++ b/configure
@@ -646,6 +646,14 @@ if [ $WITH_DESKTOP = "1" ]; then
fi
fi
+printf "Checking for cppcheck... "
+if command -v cppcheck > /dev/null; then
+have_cppcheck=1
+printf "Yes.\n"
+else
+printf "No.\n"
+fi
+
libdir_in_ldconfig=0
printf "Checking which platform we are on... "
@@ -1065,6 +1073,9 @@ zsh_completion_dir =
${ZSHCOMLETIONDIR:=\$(prefix)/share/zsh/functions/Completio
# build its own version)
HAVE_CANONICALIZE_FILE_NAME = ${have_canonicalize_file_name}
+# Whether the cppcheck static checker is available
+HAVE_CPPCHECK = ${have_cppcheck}
+
# Whether the getline function is available (if not, then notmuch will
# build its own version)
HAVE_GETLINE = ${have_getline}
--
2.14.1
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2] emacs: Add notmuch-update-search-tags
Hi On Sat, 26 Aug 2017, Vladimir Panteleev wrote: > From: Vladimir Panteleev > > Implement an option which, when enabled, causes any tag changes done > from within notmuch-emacs to instantly update matching threads in open > search buffers. I like the idea, and provided it only marks the tags as changed in the normal way, I think we could even default to on. However, I am concerned that this version does other things as well. Note these comments are from reading the patch not from testing, so please point out if I am wrong. 1) Suppose a new message arrives in the thread. Then triggering a tag change in the show buffer, (eg notmuch automatically removing an unread tag) pulls that message into original search buffer. 2) If that new message has some tags that aren't in the thread they will get marked as "added tags" 3) At least in theory I think the thread may no longer exist -- it could have merged with another thread and taken the other threads id. I think 3 is probably rare enough not to be real concern (though we should make sure we don't actually give an error). But 1 and 2 seem undesirable. Rather than refreshing the result it might make sense to fetch the tags for the actual messages in that thread (as it was when the search buffer was loaded) by using notmuch-search-find-stable-query. So something like the following, - in each notmuch-search buffer - see if thread-id thread is in that buffer (or perhaps even if there is a thread which contains message id in that buffer?) - If yes, fetch a new list of tags for the messages that buffer thinks are in that thread (using notmuch-search-find-stable-query). - Update tags appropriately. If we do that then I think the only change in these buffers is the tag updates, and exactly as if you changed those tags in the search buffer itself, the threads shown etc don't change. What do you think? Best wishes Mark ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
