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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]