Re: [PATCH v2 03/17] update-index: read --index-info with strbuf_getline_crlf()

Johannes Schindelin [off-list ref] writes:

quoted

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 7431938..a7a9a7e 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c

@@ -473,7 +473,9 @@ static void read_index_info(int line_termination)
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf uq = STRBUF_INIT;
 
-	while (strbuf_getline(&buf, stdin, line_termination) != EOF) {
+	while ((line_termination
+		? strbuf_getline_crlf(&buf, stdin)
+		: strbuf_getline(&buf, stdin, '\0')) != EOF) {
 		char *ptr, *tab;

This is a problematic change because it does not safeguard for future
introduction of a line_termination value other than LF or NUL. I believe
the safest would be to change read_index_info() to take a `nul_delimited`
parameter instead of the `line_termination` parameter first, and then
introduce that change to use strbuf_getline_crlf if !nul_delimited.

When I introduced line_termination long time ago, I wrote these
codepaths anticipating that there might be a value other than NUL
and LF that could be useful, but no useful caller that uses other
useful value has emerged, so I agree that the interface was too
broad and general for its own good.

I agree with your comments for all the changes in this series that
switch between getline and getline_delim('\0') based on the value of
line_termination; its generality is useless and we should declare
"the line termination is either NUL or LF, no other choices"; using
nul_delimited = 0/1 is a very good way to express it clearly.

Thanks.