Emily Shaffer <emilyshaffer@xxxxxxxxxx> writes:
> On Thu, Feb 20, 2020 at 12:07:46PM -0800, Junio C Hamano wrote:
>> Emily Shaffer <emilyshaffer@xxxxxxxxxx> writes:
>>
>> > +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).
