Re: [PATCH v2 02/17] check-attr, check-ignore, checkout-index: read paths with strbuf_getline_crlf()

Hi Junio,

On Wed, 16 Dec 2015, Junio C Hamano wrote:

These commands read list of paths from their standard input under
the --stdin option (in order to avoid busting limit on the length of
the command line).

When they are using text input mode (i.e. line_termination is set to
'\n'), we should try to be more friendly to our DOSsy friends and
accept lines with CRLF endings.

It is tempting to lift this logic to strbuf_getline() and not introduce
a separate strbuf_getline_crlf(), but that can lead to silent
misconversion.

This paragraph would make more sense in 01/17.

Hrm. With this much context, it is unclear that line_termination can only
ever be '\n' or '\0'. In fact, with the intention of this hunk, the
line_termination variable makes no sense any longer because its value is
not even used any longer. So something like this instead (and likewise in
check-ignore.c)?

-- snipsnap --

diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 265c9ba..605bbf9 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c

@@ -73,12 +73,13 @@ static void check_attr_stdin_paths(const char *prefix,

int cnt,
 	struct git_attr_check *check)
 {
 	struct strbuf buf, nbuf;
-	int line_termination = nul_term_line ? 0 : '\n';
 
 	strbuf_init(&buf, 0);
 	strbuf_init(&nbuf, 0);
-	while (strbuf_getline(&buf, stdin, line_termination) != EOF) {
-		if (line_termination && buf.buf[0] == '"') {
+	while ((nul_term_line
+			? strbuf_getline(&buf, stdin, '\0')
+			: strbuf_getline_crlf(&buf, stdin)) != EOF) {
+		if (!nul_term_line && buf.buf[0] == '"') {
 			strbuf_reset(&nbuf);
 			if (unquote_c_style(&nbuf, buf.buf, NULL))
 				die("line is badly quoted");