vreportf: report to arbitrary filehandles
authorJeff King <peff@peff.net>
Tue, 11 Aug 2015 18:06:15 +0000 (14:06 -0400)
committerJunio C Hamano <gitster@pobox.com>
Tue, 11 Aug 2015 21:24:50 +0000 (14:24 -0700)
The vreportf function always goes to stderr, but run-command
wants child errors to go to the parent's original stderr. To
solve this, commit a5487dd duplicates the stderr fd and
installs die and error handlers to direct the output
appropriately (which later turned into the vwritef
function). This has two downsides, though:

- we make multiple calls to write(), which contradicts the
"write at once" logic from d048a96 (print
warning/error/fatal messages in one shot, 2007-11-09).

- the custom handlers basically duplicate the normal
handlers. They're only a few lines of code, but we
should not have to repeat the magic "exit(128)", for
example.

We can solve the first by using fdopen() on the duplicated
descriptor. We can't pass this to vreportf, but we could
introduce a new vreportf_to to handle it.

However, to fix the second problem, we instead introduce a
new "set_error_handle" function, which lets the normal
vreportf calls output to a handle besides stderr. Thus we
can get rid of our custom handlers entirely, and just ask
the regular handlers to output to our new descriptor.

And as vwritef has no more callers, it can just go away.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-compat-util.h
run-command.c
usage.c
index c6d391f86490b94e4df667d64d54323e1915c477..076461e8c8d3cd2051e84060c2ca405c3a3577ac 100644 (file)
@@ -389,7 +389,6 @@ struct strbuf;
 
 /* General helper functions */
 extern void vreportf(const char *prefix, const char *err, va_list params);
-extern void vwritef(int fd, const char *prefix, const char *err, va_list params);
 extern NORETURN void usage(const char *err);
 extern NORETURN void usagef(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2)));
@@ -425,6 +424,7 @@ static inline int const_error(void)
 extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
 extern void set_error_routine(void (*routine)(const char *err, va_list params));
 extern void set_die_is_recursing_routine(int (*routine)(void));
+extern void set_error_handle(FILE *);
 
 extern int starts_with(const char *str, const char *prefix);
 
index 4d73e90fad159184bfdd204b82dd8637ad28a955..0d01671c1ff26ccfe04481713a4071c321d8ab11 100644 (file)
@@ -200,7 +200,6 @@ static int execv_shell_cmd(const char **argv)
 #endif
 
 #ifndef GIT_WINDOWS_NATIVE
-static int child_err = 2;
 static int child_notifier = -1;
 
 static void notify_parent(void)
@@ -212,17 +211,6 @@ static void notify_parent(void)
         */
        xwrite(child_notifier, "", 1);
 }
-
-static NORETURN void die_child(const char *err, va_list params)
-{
-       vwritef(child_err, "fatal: ", err, params);
-       exit(128);
-}
-
-static void error_child(const char *err, va_list params)
-{
-       vwritef(child_err, "error: ", err, params);
-}
 #endif
 
 static inline void set_cloexec(int fd)
@@ -362,11 +350,10 @@ int start_command(struct child_process *cmd)
                 * in subsequent call paths use the parent's stderr.
                 */
                if (cmd->no_stderr || need_err) {
-                       child_err = dup(2);
+                       int child_err = dup(2);
                        set_cloexec(child_err);
+                       set_error_handle(fdopen(child_err, "w"));
                }
-               set_die_routine(die_child);
-               set_error_routine(error_child);
 
                close(notify_pipe[0]);
                set_cloexec(notify_pipe[1]);
diff --git a/usage.c b/usage.c
index ed146453cabeb9e82bef0c5610cda47d1b2a0b1c..e4fa6d2f0315c6f5ae63c97d83bfab3a14e279b2 100644 (file)
--- a/usage.c
+++ b/usage.c
@@ -6,23 +6,14 @@
 #include "git-compat-util.h"
 #include "cache.h"
 
+static FILE *error_handle;
+
 void vreportf(const char *prefix, const char *err, va_list params)
 {
        char msg[4096];
+       FILE *fh = error_handle ? error_handle : stderr;
        vsnprintf(msg, sizeof(msg), err, params);
-       fprintf(stderr, "%s%s\n", prefix, msg);
-}
-
-void vwritef(int fd, const char *prefix, const char *err, va_list params)
-{
-       char msg[4096];
-       int len = vsnprintf(msg, sizeof(msg), err, params);
-       if (len > sizeof(msg))
-               len = sizeof(msg);
-
-       write_in_full(fd, prefix, strlen(prefix));
-       write_in_full(fd, msg, len);
-       write_in_full(fd, "\n", 1);
+       fprintf(fh, "%s%s\n", prefix, msg);
 }
 
 static NORETURN void usage_builtin(const char *err, va_list params)
@@ -76,6 +67,11 @@ void set_die_is_recursing_routine(int (*routine)(void))
        die_is_recursing = routine;
 }
 
+void set_error_handle(FILE *fh)
+{
+       error_handle = fh;
+}
+
 void NORETURN usagef(const char *err, ...)
 {
        va_list params;