Skip to content

Commit 3b331e9

Browse files
peffgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent a17c56c commit 3b331e9

File tree

3 files changed

+12
-29
lines changed

3 files changed

+12
-29
lines changed

git-compat-util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,6 @@ struct strbuf;
389389

390390
/* General helper functions */
391391
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);
393392
extern NORETURN void usage(const char *err);
394393
extern NORETURN void usagef(const char *err, ...) __attribute__((format (printf, 1, 2)));
395394
extern NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2)));
@@ -425,6 +424,7 @@ static inline int const_error(void)
425424
extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
426425
extern void set_error_routine(void (*routine)(const char *err, va_list params));
427426
extern void set_die_is_recursing_routine(int (*routine)(void));
427+
extern void set_error_handle(FILE *);
428428

429429
extern int starts_with(const char *str, const char *prefix);
430430

run-command.c

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,6 @@ static int execv_shell_cmd(const char **argv)
200200
#endif
201201

202202
#ifndef GIT_WINDOWS_NATIVE
203-
static int child_err = 2;
204203
static int child_notifier = -1;
205204

206205
static void notify_parent(void)
@@ -212,17 +211,6 @@ static void notify_parent(void)
212211
*/
213212
xwrite(child_notifier, "", 1);
214213
}
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-
}
226214
#endif
227215

228216
static inline void set_cloexec(int fd)
@@ -362,11 +350,10 @@ int start_command(struct child_process *cmd)
362350
* in subsequent call paths use the parent's stderr.
363351
*/
364352
if (cmd->no_stderr || need_err) {
365-
child_err = dup(2);
353+
int child_err = dup(2);
366354
set_cloexec(child_err);
355+
set_error_handle(fdopen(child_err, "w"));
367356
}
368-
set_die_routine(die_child);
369-
set_error_routine(error_child);
370357

371358
close(notify_pipe[0]);
372359
set_cloexec(notify_pipe[1]);

usage.c

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,14 @@
66
#include "git-compat-util.h"
77
#include "cache.h"
88

9+
static FILE *error_handle;
10+
911
void vreportf(const char *prefix, const char *err, va_list params)
1012
{
1113
char msg[4096];
14+
FILE *fh = error_handle ? error_handle : stderr;
1215
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);
2617
}
2718

2819
static NORETURN void usage_builtin(const char *err, va_list params)
@@ -76,6 +67,11 @@ void set_die_is_recursing_routine(int (*routine)(void))
7667
die_is_recursing = routine;
7768
}
7869

70+
void set_error_handle(FILE *fh)
71+
{
72+
error_handle = fh;
73+
}
74+
7975
void NORETURN usagef(const char *err, ...)
8076
{
8177
va_list params;

0 commit comments

Comments
 (0)