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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 4, 2016, 12:25 UTC
Message-ID: <alpine.DEB.2.20.1601041258280.14434@virtualbox>
In-Reply-To: <1450303398-25900-3-git-send-email-gitster@pobox.com>

Hi Junio,

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

Show 11 quoted lines

> 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.

Show 15 quoted lines

> diff --git a/builtin/check-attr.c b/builtin/check-attr.c
> index 265c9ba..4c44d8f 100644
> --- a/builtin/check-attr.c
> +++ b/builtin/check-attr.c
> @@ -77,7 +77,9 @@ static void check_attr_stdin_paths(const char *prefix, int cnt,
>  
>  	strbuf_init(&buf, 0);
>  	strbuf_init(&nbuf, 0);
> -	while (strbuf_getline(&buf, stdin, line_termination) != EOF) {
> +	while ((line_termination
> +		? strbuf_getline_crlf(&buf, stdin)
> +		: strbuf_getline(&buf, stdin, '\0')) != EOF) {
>  		if (line_termination && buf.buf[0] == '"') {
>  			strbuf_reset(&nbuf);
>  			if (unquote_c_style(&nbuf, buf.buf, NULL))

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");