On Thu, Jan 16, 2025 at 09:22:54AM -0800, Junio C Hamano wrote:
> >> - fflush(stderr);
> >> - write_in_full(2, msg, p - msg);
> >> + if (fd == 2)
> >> + fflush(stderr);
> >> + write_in_full(fd, msg, p - msg);
> >> +}
> >
> > Gross. :) I think the existing code is conceptually:
> >
> > write_in_full(fileno(stderr), msg, p - msg);
> >
> > In which case vfreportf() could just take a FILE*, flush it and then
> > write.
>
> Sure, but these "stderr" are real error reporting that need to stay
> to be stderr, and flush needs to be done only when our true payload
> goes to fd#2 and I do not think these fflush() are about stdio calls
> made by the caller _before_ it called this function. It may become
> a bit tricky to read the resulting code if we pass "FILE *".
I think the flush _is_ about earlier stdio calls in the process; that's
what 116d1fa6c6 (vreportf(): avoid relying on stdio buffering,
2019-10-30) says. I do think it's unlikely for the process to write to
stdout() before processing "-h", but it seems like we should do the
safer thing as a general principle, unless it ends up hard to do so.
You said "may become a bit tricky" above, but unless I'm missing
something, it's just:
---
usage.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/usage.c b/usage.c
index d9f323a0bd..533d296e41 100644
--- a/usage.c
+++ b/usage.c
@@ -8,7 +8,7 @@
#include "gettext.h"
#include "trace2.h"
-static void vfdreportf(int fd, const char *prefix, const char *err, va_list params)
+static void vfreportf(FILE *fh, const char *prefix, const char *err, va_list params)
{
char msg[4096];
char *p, *pend = msg + sizeof(msg);
@@ -32,14 +32,13 @@ static void vfdreportf(int fd, const char *prefix, const char *err, va_list para
}
*(p++) = '\n'; /* we no longer need a NUL */
- if (fd == 2)
- fflush(stderr);
- write_in_full(fd, msg, p - msg);
+ fflush(fh);
+ write_in_full(fileno(fh), msg, p - msg);
}
static void vreportf(const char *prefix, const char *err, va_list params)
{
- vfdreportf(2, prefix, err, params);
+ vfreportf(stderr, prefix, err, params);
}
static NORETURN void usage_builtin(const char *err, va_list params)
@@ -184,7 +183,7 @@ static void show_usage_if_asked_helper(const char *err, ...)
va_list params;
va_start(params, err);
- vfdreportf(1, _("usage: "), err, params);
+ vfreportf(stdout, _("usage: "), err, params);
va_end(params);
exit(129);
}
