Re: What's cooking in git.git (Jan 2016, #02; Mon, 11)

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> My plan is to use a function pointer to switch between them.  A code
> like the above in practice look more like
>
> (1) there is a config/option parser that sets line_terminator that
>     is typically a file-scope global.
>
> 	if (z_option)
>         	line_terminator = '\0';
> 	else
>         	line_terminator = '\n';
>
> (2) the callsite calls getline with it
>
> 	strbuf_getline(..., line_terminator);
>
> So we can introduce a file-scope global, (*getline_fn)(), and then
> tweak (1) by removing line_terminator and replacing the assignment
> with an assignment to getline_fn.

And after doing the obvious wholesale replacement on callers that
hardcode either '\n' or '\0' with this:

#!/bin/sh
perl -i -p -e '
	s/strbuf_getline\((.*?), '\''\\n'\''\)/strbuf_getline_lf($1)/g;
	s/strbuf_getline\((.*?), '\''\\0'\''\)/strbuf_getline_nul($1)/g;
' "$@"

the only direct callers of strbuf_getline() that remain in the tree
are check-attr, check-ignore, checkout-index and mktree.

And the conversion of mktree would look like this.  I might further
tweak it to rename and flip the polarity of lf_lines to nul_lines,
but I do not think that matters very much.

 builtin/mktree.c | 15 +++++++++------
 strbuf.h         |  3 +++
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/builtin/mktree.c b/builtin/mktree.c
index a964d6b..7633d35 100644
--- a/builtin/mktree.c
+++ b/builtin/mktree.c
@@ -65,7 +65,7 @@ static const char *mktree_usage[] = {
 	NULL
 };
 
-static void mktree_line(char *buf, size_t len, int line_termination, int allow_missing)
+static void mktree_line(char *buf, size_t len, int lf_lines, int allow_missing)
 {
 	char *ptr, *ntr;
 	unsigned mode;
@@ -97,7 +97,7 @@ static void mktree_line(char *buf, size_t len, int line_termination, int allow_m
 	*ntr++ = 0; /* now at the beginning of SHA1 */
 
 	path = ntr + 41;  /* at the beginning of name */
-	if (line_termination && path[0] == '"') {
+	if (lf_lines && path[0] == '"') {
 		struct strbuf p_uq = STRBUF_INIT;
 		if (unquote_c_style(&p_uq, path, NULL))
 			die("invalid quoting");
@@ -141,13 +141,14 @@ int cmd_mktree(int ac, const char **av, const char *prefix)
 {
 	struct strbuf sb = STRBUF_INIT;
 	unsigned char sha1[20];
-	int line_termination = '\n';
+	int lf_lines = 1;
 	int allow_missing = 0;
 	int is_batch_mode = 0;
 	int got_eof = 0;
+	strbuf_getline_fn getline_fn;
 
 	const struct option option[] = {
-		OPT_SET_INT('z', NULL, &line_termination, N_("input is NUL terminated"), '\0'),
+		OPT_SET_INT('z', NULL, &lf_lines, N_("input is NUL terminated"), '\0'),
 		OPT_SET_INT( 0 , "missing", &allow_missing, N_("allow missing objects"), 1),
 		OPT_SET_INT( 0 , "batch", &is_batch_mode, N_("allow creation of more than one tree"), 1),
 		OPT_END()
@@ -155,9 +156,11 @@ int cmd_mktree(int ac, const char **av, const char *prefix)
 
 	ac = parse_options(ac, av, prefix, option, mktree_usage, 0);
 
+	getline_fn = lf_lines ? strbuf_getline_lf : strbuf_getline_nul;
+
 	while (!got_eof) {
 		while (1) {
-			if (strbuf_getline(&sb, stdin, line_termination) == EOF) {
+			if (getline_fn(&sb, stdin) == EOF) {
 				got_eof = 1;
 				break;
 			}
@@ -167,7 +170,7 @@ int cmd_mktree(int ac, const char **av, const char *prefix)
 					break;
 				die("input format error: (blank line only valid in batch mode)");
 			}
-			mktree_line(sb.buf, sb.len, line_termination, allow_missing);
+			mktree_line(sb.buf, sb.len, lf_lines, allow_missing);
 		}
 		if (is_batch_mode && got_eof && used < 1) {
 			/*
diff --git a/strbuf.h b/strbuf.h
index db053be..9c8d715 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -405,6 +405,9 @@ static inline int strbuf_getline_nul(struct strbuf *sb, FILE *fp)
 	return strbuf_getline(sb, fp, '\0');
 }
 
+typedef int (*strbuf_getline_fn)(struct strbuf *, FILE *);
+
+
 /**
  * Like `strbuf_getline`, but keeps the trailing terminator (if
  * any) in the buffer.