Re: [PATCH 2/7] nfv?asprintf are broken without va_copy, workaround them.
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:35
Pierre Habouzit [off-list ref] writes:
* drop nfasprintf. * move nfvasprintf into imap-send.c back, and let it work on a 8k buffer, and die() in case of overflow. It should be enough for imap commands, if someone cares about imap-send, he's welcomed to fix it properly. * replace nfvasprintf use in merge-recursive with a copy of the strbuf_addf logic, it's one place, we'll live with it. To ease the change, output_buffer string list is replaced with a strbuf ;)
While I'd agree with all of the above,
* rework trace.c API's so that only one of the trace functions takes a vararg. It's used to format strerror()s and git command names, it should never be more than a few octets long, let it work on a 8k static buffer with vsnprintf or die loudly.
and I'd agree with this in principle, there is a minor nit with the implementation and use in trace.c. E.g.
quoted hunk
diff --git a/exec_cmd.c b/exec_cmd.c index 9b74ed2..c0f954e 100644 --- a/exec_cmd.c +++ b/exec_cmd.c@@ -97,7 +97,8 @@ int execv_git_cmd(const char **argv) tmp = argv[0]; argv[0] = git_command; - trace_argv_printf(argv, -1, "trace: exec:"); + trace_printf("trace: exec:"); + trace_argv(argv, -1);
This used to be a single call into trace.c which would format a single string to write(2) out. Now these two messages go through separate write(2) and can be broken up. I think the atomicity of the log/trace message was the primary reason the original had such a strange calling convention.