v3 of --stderr=FILE patches

v3 of --stderr=FILE patches

[Tomi Ollila]

This is v3 of id:1369557954-13439-1-git-send-email-tomi.ollila@iki.fi

In id:m2a9nfr4tb.fsf@guru.guru-group.fi I suggested to use 0600 as
permission bits so that files written to /tmp would not have write
bits set in any case. That would have been inconsistent what is
normally expected in redirection cases. The problem I described
is easily avoided in emacs as (make-temp-file "nmerr") will create
file "/tmp/nmerrXXXXXX" with permissions 0600 -- and writing to
that already-created file will not change its permissions. Writing
elsewhere is usually covered with directory permissions.


Diffdiff from v2 is shown below.

diff --git a/NEWS b/NEWS
index 990b038..80abd97 100644
--- a/NEWS
+++ b/NEWS
@@ -38,8 +38,7 @@ Top level option to specify configuration file
 Top level option to redirect writes to stderr
 
   With `notmuch --stderr=FILE` all writes to stderr are redirected to
-  the specified file. If the file name is a plain '-', stderr is
-  written to stdout.
+  the specified file. If FILE is '-', stderr is redirected to stdout.
 
 Deprecated commands "part" and "search-tags" are removed.
 
diff --git a/man/man1/notmuch.1 b/man/man1/notmuch.1
index fbd575a..f5ca0ad 100644
--- a/man/man1/notmuch.1
+++ b/man/man1/notmuch.1
@@ -83,7 +83,7 @@ configuration file specified by ${NOTMUCH_CONFIG}.
 .B \-\-stderr=FILE
 
 Redirect all writes to stderr to the specified file.
-If the file name is a plain '-', stderr is written to stdout.
+If FILE is '-', stderr is redirected to stdout.
 .RE
 
 .SH COMMANDS
diff --git a/notmuch.c b/notmuch.c
index 654a568..15e90c8 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -260,7 +260,7 @@ redirect_stderr (const char * stderr_file)
 	    return 1;
 	}
     } else {
-	int fd = open (stderr_file, O_WRONLY|O_CREAT|O_APPEND, 0644);
+	int fd = open (stderr_file, O_WRONLY|O_CREAT|O_TRUNC, 0666);
 	if (fd < 0) {
 	    fprintf (stderr, "Error: Cannot redirect stderr to '%s': %s\n",
 		     stderr_file, strerror (errno));


From Tomi Ollila <tomi.ollila@iki.fi> # This line is ignored.

[PATCH v3 1/4] cli: add global option --stderr=FILE

[Tomi Ollila]

With this option all writes to stderr are redirected to the spesified
FILE (or to stdout on case FILE is '-'). This is immediately useful
in emacs interface as some of its exec intefaces do not provide
separation of stdout and stderr.
---
 notmuch-client.h |  1 +
 notmuch.c        | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/notmuch-client.h b/notmuch-client.h
index 45749a6..4a3c7ac 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -54,6 +54,7 @@ typedef GMimeCipherContext notmuch_crypto_context_t;
 #include <sys/stat.h>
 #include <sys/time.h>
 #include <unistd.h>
+#include <fcntl.h>
 #include <dirent.h>
 #include <errno.h>
 #include <signal.h>
diff --git a/notmuch.c b/notmuch.c
index f51a84f..15e90c8 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -251,6 +251,32 @@ notmuch_command (notmuch_config_t *config,
     return 0;
 }
 
+static int
+redirect_stderr (const char * stderr_file)
+{
+    if (strcmp (stderr_file, "-") == 0) {
+	if (dup2 (STDOUT_FILENO, STDERR_FILENO) < 0) {
+	    perror ("dup2");
+	    return 1;
+	}
+    } else {
+	int fd = open (stderr_file, O_WRONLY|O_CREAT|O_TRUNC, 0666);
+	if (fd < 0) {
+	    fprintf (stderr, "Error: Cannot redirect stderr to '%s': %s\n",
+		     stderr_file, strerror (errno));
+	    return 1;
+	}
+	if (fd != STDERR_FILENO) {
+	    if (dup2 (fd, STDERR_FILENO) < 0) {
+		perror ("dup2");
+		return 1;
+	    }
+	    close (fd);
+	}
+    }
+    return 0;
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -259,6 +285,7 @@ main (int argc, char *argv[])
     const char *command_name = NULL;
     command_t *command;
     char *config_file_name = NULL;
+    char *stderr_file = NULL;
     notmuch_config_t *config;
     notmuch_bool_t print_help=FALSE, print_version=FALSE;
     int opt_index;
@@ -268,6 +295,7 @@ main (int argc, char *argv[])
 	{ NOTMUCH_OPT_BOOLEAN, &print_help, "help", 'h', 0 },
 	{ NOTMUCH_OPT_BOOLEAN, &print_version, "version", 'v', 0 },
 	{ NOTMUCH_OPT_STRING, &config_file_name, "config", 'c', 0 },
+	{ NOTMUCH_OPT_STRING, &stderr_file, "stderr", '\0', 0 },
 	{ 0, 0, 0, 0, 0 }
     };
 
@@ -287,6 +315,10 @@ main (int argc, char *argv[])
 	return 1;
     }
 
+    if (stderr_file && redirect_stderr (stderr_file) != 0) {
+	/* error already printed */
+	return 1;
+    }
     if (print_help)
 	return notmuch_help_command (NULL, argc - 1, &argv[1]);
 
-- 
1.8.1.4

[PATCH v3 2/4] test: added --stderr=FILE tests

[Tomi Ollila]

--stderr=FILE tests were added to test/help-test as it is the one
doing most global option testing. Also, it was simplest to test
this new option using `notmuch help` command.
---
 test/help-test | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/test/help-test b/test/help-test
index f7df725..bd0111c 100755
--- a/test/help-test
+++ b/test/help-test
@@ -9,4 +9,13 @@ test_expect_success 'notmuch help' 'notmuch help'
 test_expect_success 'notmuch help tag' 'notmuch help tag'
 test_expect_success 'notmuch --version' 'notmuch --version'
 
+test_begin_subtest "notmuch --stderr=stderr help %"
+notmuch --stderr=stderr help %
+test_expect_equal "$(cat stderr)" "
+Sorry, % is not a known command. There's not much I can do to help."
+
+test_begin_subtest "notmuch --stderr=- help %"
+test_expect_equal "$(notmuch --stderr=- help %)" "
+Sorry, % is not a known command. There's not much I can do to help."
+
 test_done
-- 
1.8.1.4

[PATCH v3 3/4] man: documented --stderr=FILE in notmuch.1 manual page

[Tomi Ollila]

---
 man/man1/notmuch.1 | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/man/man1/notmuch.1 b/man/man1/notmuch.1
index 033cc10..f5ca0ad 100644
--- a/man/man1/notmuch.1
+++ b/man/man1/notmuch.1
@@ -76,7 +76,14 @@ Print the installed version of notmuch, and exit.
 
 Specify the configuration file to use. This overrides any
 configuration file specified by ${NOTMUCH_CONFIG}.
+.RE
+
+.RS 4
+.TP 4
+.B \-\-stderr=FILE
 
+Redirect all writes to stderr to the specified file.
+If FILE is '-', stderr is redirected to stdout.
 .RE
 
 .SH COMMANDS
-- 
1.8.1.4

[PATCH v3 4/4] NEWS: added information about new --stderr=FILE top level option

[Tomi Ollila]

---
 NEWS | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/NEWS b/NEWS
index a7f2ec6..80abd97 100644
--- a/NEWS
+++ b/NEWS
@@ -35,6 +35,11 @@ Top level option to specify configuration file
   It's now possible to specify the configuration file to use on the
   command line using the `notmuch --config=FILE` option.
 
+Top level option to redirect writes to stderr
+
+  With `notmuch --stderr=FILE` all writes to stderr are redirected to
+  the specified file. If FILE is '-', stderr is redirected to stdout.
+
 Deprecated commands "part" and "search-tags" are removed.
 
 Bash command-line completion
-- 
1.8.1.4

Re: v3 of --stderr=FILE patches

[Mark Walters]

Just confirming LGTM +1 for this version too.

Best wishes

Mark

Tomi Ollila <tomi.ollila@iki.fi> writes:

> This is v3 of id:1369557954-13439-1-git-send-email-tomi.ollila@iki.fi
>
> In id:m2a9nfr4tb.fsf@guru.guru-group.fi I suggested to use 0600 as
> permission bits so that files written to /tmp would not have write
> bits set in any case. That would have been inconsistent what is
> normally expected in redirection cases. The problem I described
> is easily avoided in emacs as (make-temp-file "nmerr") will create
> file "/tmp/nmerrXXXXXX" with permissions 0600 -- and writing to
> that already-created file will not change its permissions. Writing
> elsewhere is usually covered with directory permissions.
>
>
> Diffdiff from v2 is shown below.
>
> diff --git a/NEWS b/NEWS
> index 990b038..80abd97 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -38,8 +38,7 @@ Top level option to specify configuration file
>  Top level option to redirect writes to stderr
>  
>    With `notmuch --stderr=FILE` all writes to stderr are redirected to
> -  the specified file. If the file name is a plain '-', stderr is
> -  written to stdout.
> +  the specified file. If FILE is '-', stderr is redirected to stdout.
>  
>  Deprecated commands "part" and "search-tags" are removed.
>  
> diff --git a/man/man1/notmuch.1 b/man/man1/notmuch.1
> index fbd575a..f5ca0ad 100644
> --- a/man/man1/notmuch.1
> +++ b/man/man1/notmuch.1
> @@ -83,7 +83,7 @@ configuration file specified by ${NOTMUCH_CONFIG}.
>  .B \-\-stderr=FILE
>  
>  Redirect all writes to stderr to the specified file.
> -If the file name is a plain '-', stderr is written to stdout.
> +If FILE is '-', stderr is redirected to stdout.
>  .RE
>  
>  .SH COMMANDS
> diff --git a/notmuch.c b/notmuch.c
> index 654a568..15e90c8 100644
> --- a/notmuch.c
> +++ b/notmuch.c
> @@ -260,7 +260,7 @@ redirect_stderr (const char * stderr_file)
>  	    return 1;
>  	}
>      } else {
> -	int fd = open (stderr_file, O_WRONLY|O_CREAT|O_APPEND, 0644);
> +	int fd = open (stderr_file, O_WRONLY|O_CREAT|O_TRUNC, 0666);
>  	if (fd < 0) {
>  	    fprintf (stderr, "Error: Cannot redirect stderr to '%s': %s\n",
>  		     stderr_file, strerror (errno));
>
>
> From Tomi Ollila <tomi.ollila@iki.fi> # This line is ignored.
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: v3 of --stderr=FILE patches

[Austin Clements]

LGTM.

Quoth Tomi Ollila on May 28 at  9:38 pm:
> This is v3 of id:1369557954-13439-1-git-send-email-tomi.ollila@iki.fi
> 
> In id:m2a9nfr4tb.fsf@guru.guru-group.fi I suggested to use 0600 as
> permission bits so that files written to /tmp would not have write
> bits set in any case. That would have been inconsistent what is
> normally expected in redirection cases. The problem I described
> is easily avoided in emacs as (make-temp-file "nmerr") will create
> file "/tmp/nmerrXXXXXX" with permissions 0600 -- and writing to
> that already-created file will not change its permissions. Writing
> elsewhere is usually covered with directory permissions.

Re: v3 of --stderr=FILE patches

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> This is v3 of id:1369557954-13439-1-git-send-email-tomi.ollila@iki.fi
>

pushed, with one spelling correction in a commit message.

d