vreportf: report to arbitrary filehandles

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>

Jeff King committed Aug 11, 2015 at 14:06 UTC 3b331e92671469614662830402103848a8004b97
3 files changed +12 -29
git-compat-util.h
+1 -1
@@ -389,7 +389,6 @@ struct strbuf;
389
390 /* General helper functions */
391 extern void vreportf(const char *prefix, const char *err, va_list params);
392 -extern void vwritef(int fd, const char *prefix, const char *err, va_list params);
392 extern NORETURN void usage(const char *err);
393 extern NORETURN void usagef(const char *err, ...) __attribute__((format (printf, 1, 2)));
394 extern NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2)));
@@ -425,6 +424,7 @@ static inline int const_error(void)
424 extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
425 extern void set_error_routine(void (*routine)(const char *err, va_list params));
426 extern void set_die_is_recursing_routine(int (*routine)(void));
427 +extern void set_error_handle(FILE *);
428
429 extern int starts_with(const char *str, const char *prefix);
430
run-command.c
+2 -15
@@ -200,7 +200,6 @@ static int execv_shell_cmd(const char **argv)
200 #endif
201
202 #ifndef GIT_WINDOWS_NATIVE
203 -static int child_err = 2;
203 static int child_notifier = -1;
204
205 static void notify_parent(void)
@@ -212,17 +211,6 @@ static void notify_parent(void)
211 */
212 xwrite(child_notifier, "", 1);
213 }
215 -
216 -static NORETURN void die_child(const char *err, va_list params)
217 -{
218 - vwritef(child_err, "fatal: ", err, params);
219 - exit(128);
220 -}
221 -
222 -static void error_child(const char *err, va_list params)
223 -{
224 - vwritef(child_err, "error: ", err, params);
225 -}
214 #endif
215
216 static inline void set_cloexec(int fd)
@@ -362,11 +350,10 @@ fail_pipe:
350 * in subsequent call paths use the parent's stderr.
351 */
352 if (cmd->no_stderr || need_err) {
365 - child_err = dup(2);
353 + int child_err = dup(2);
354 set_cloexec(child_err);
355 + set_error_handle(fdopen(child_err, "w"));
356 }
368 - set_die_routine(die_child);
369 - set_error_routine(error_child);
357
358 close(notify_pipe[0]);
359 set_cloexec(notify_pipe[1]);
usage.c
+9 -13
@@ -6,23 +6,14 @@
6 #include "git-compat-util.h"
7 #include "cache.h"
8
9 +static FILE *error_handle;
10 +
11 void vreportf(const char *prefix, const char *err, va_list params)
12 {
13 char msg[4096];
14 + FILE *fh = error_handle ? error_handle : stderr;
15 vsnprintf(msg, sizeof(msg), err, params);
13 - fprintf(stderr, "%s%s\n", prefix, msg);
14 -}
15 -
16 -void vwritef(int fd, const char *prefix, const char *err, va_list params)
17 -{
18 - char msg[4096];
19 - int len = vsnprintf(msg, sizeof(msg), err, params);
20 - if (len > sizeof(msg))
21 - len = sizeof(msg);
22 -
23 - write_in_full(fd, prefix, strlen(prefix));
24 - write_in_full(fd, msg, len);
25 - write_in_full(fd, "\n", 1);
16 + fprintf(fh, "%s%s\n", prefix, msg);
17 }
18
19 static NORETURN void usage_builtin(const char *err, va_list params)
@@ -76,6 +67,11 @@ void set_die_is_recursing_routine(int (*routine)(void))
67 die_is_recursing = routine;
68 }
69
70 +void set_error_handle(FILE *fh)
71 +{
72 + error_handle = fh;
73 +}
74 +
75 void NORETURN usagef(const char *err, ...)
76 {
77 va_list params;