Re: [PATCH v3 08/15] merge-ort: allow update messages to be written to different file stream

> >> I would get it if the point was to actually use the full usage.c
> >> machinery, but we're just either calling warning(), or printing a
> >> formatted string to a file FILE *. There's no need to go through usage.c
> >> for that, and adding an API to it that behaves like this new
> >> warning_fp() is really confusing.
> >
> > Because the formatted string being printed to the file won't have the
> > same "warning: " prefix that is normally added to stuff in usage?
>
> But the pre-image doesn't add that either. We're just calling
> vfprintf(), not our own vreportf().

> > That's a fair point; that does have a bit of a consistency problem.
> > And I'd rather the messages were consistent regardless of where they
> > are printed.
>
> I think that makes sense, that's why I added die_message() recently. If
> you meant to print a "warning: " prefix I think it would also be fine in
> this case to just do it inline. See prior art at:
>
>     git grep '"(fatal|error|warning):' -- '*.c'

> >> diff --git a/diff.c b/diff.c
> >> index 28368110147..4cf67e93dea 100644
> >> --- a/diff.c
> >> +++ b/diff.c
> >> @@ -6377,14 +6377,21 @@ static const char rename_limit_advice[] =
> >>  N_("you may want to set your %s variable to at least "
> >>     "%d and retry the command.");
> >>
> >> +#define warning_fp(out, fmt, ...) do { \
> >> +       if (out == stderr) \
> >> +               warning(fmt, __VA_ARGS__); \
> >> +       else \
> >> +               fprintf(out, fmt, __VA_ARGS__); \
> >> +} while (0)
> >> +
> >>  void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc,
> >>                             FILE *out)
> >>  {
> >>         fflush(stdout);
> >>         if (degraded_cc)
> >> -               warning_fp(out, _(degrade_cc_to_c_warning));
> >> +               warning_fp(out, _(degrade_cc_to_c_warning), NULL);
> >>         else if (needed)
> >> -               warning_fp(out, _(rename_limit_warning));
> >> +               warning_fp(out, _(rename_limit_warning), NULL);
> >
> > Why do the only callers have a NULL parameter here?  Is this one of
> > those va_list/va_args things I never bothered to properly learn?
>
> That's wrong (I blame tiredness last night),an actual working version is
> produced below. Clang accepted my broken code, but gcc rightly yells
> about it:

> Note that both your pre-image, my macro version and Johannes's
> linked-to-above are technically buggy in that they treat a
> non-formatting format as a formatting format. I.e. we should use
> warning("%s", msg) in that case, not warning(msg).
>
> See 927dc330705 (advice.h: add missing __attribute__((format)) & fix
> usage, 2021-07-13) for a similar bug/fix.