Re: [PATCH v8 04/15] bugreport: gather git version and build info

Emily Shaffer [off-list ref] writes:

On Thu, Feb 20, 2020 at 12:07:46PM -0800, Junio C Hamano wrote:

quoted

Emily Shaffer [off-list ref] writes:

quoted

+static void get_system_info(struct strbuf *sys_info)
+{
+	/* get git version from native cmd */
+	strbuf_addstr(sys_info, "git version:\n");
+	get_version_info(sys_info, 1);
+	strbuf_complete_line(sys_info);

It is a bit curious use of "don't do anything if sys_info ends with
a complete line, but complete it if it ends with an imcomplete
line".  That tells the readers that we do not know what
get_version_info() will do (now or in the future) to its output
buffer.
 ...
So, was the strbuf_complete_line() merely defensive programming?  It
may deserve a comment if it will stay there.

It was meant defensively, here and elsewhere in the series. I figured
that for something like this, which is mostly bounded by human writing
in an editor and then by file IO, spurious string-checking was not such
a big deal.

Are you suggesting to comment around the strbuf_complete_line() calls,
or to comment around get_version_info() that it should end in newline?

I meant a comment in get_system_info() next to the use of this
particular use of strbuf_complete_line(), if the use stays there.

But after reading the whole series through, I saw no need to use
strbuf_complete_line() in the first place, as there is no source of
input that is not under our control (if we do not count sloppy
programming, that is).