Re: [PATCH 5/6] bugreport.c: tweak cmd_bugreport() to use __attribute__((printf))

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> As noted in the preceding commit we had no __attribute__((format))
> check for strbuf_addftime(), and furthermore had no in-tree user that
> passed it a fixed string.
>
> Let's tweak this codepath added in 238b439d698 (bugreport: add tool to
> generate debugging info, 2020-04-16) to make use of the compile-time
> check.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/bugreport.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/bugreport.c b/builtin/bugreport.c
> index 9915a5841de..02edfb9a047 100644
> --- a/builtin/bugreport.c
> +++ b/builtin/bugreport.c
> @@ -127,7 +127,7 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
>  	time_t now = time(NULL);
>  	struct tm tm;
>  	char *option_output = NULL;
> -	char *option_suffix = "%Y-%m-%d-%H%M";
> +	char *option_suffix = NULL;
> ...
> +	if (option_suffix)
> +		strbuf_addftime(&report_path, option_suffix, localtime_r(&now, &tm), 0, 0);
> +	else
> +		strbuf_addftime(&report_path, "%Y-%m-%d-%H%M", localtime_r(&now, &tm), 0, 0);
>  	strbuf_addstr(&report_path, ".txt");
>  
>  	switch (safe_create_leading_directories(report_path.buf)) {

The use of "the variable option_suffix whose default value is
'%Y-%m-%d-%H%M'" may happen to be only to generate report_path in
the current code, but there is little reason to expect that will
stay to be true.  With this change, however, future developers has
to know that option_suffix may not have its default value in it and
they need to do the "if it is NULL, then use the default", and
without any benefit of proprocessor macro.

And in exchange of it, we gain what?  Letting some compilers ensure
that "%Y-%m-%d-%H%M" is a valid strftime format string?  That does
not sound all that much interesting.

This step I am not impressed all that much, even though the earlier
clean-up steps all looked sensible.