replace strbuf_expand() with strbuf_expand_step()

Avoid the overhead of passing context to a callback function of
strbuf_expand() by using strbuf_expand_step() in a loop instead.  It
requires explicit handling of %% and unrecognized placeholders, but is
simpler, more direct and avoids void pointers.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

7 files changed, 169 insertions, 272 deletions

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 0bafc14e6c..424f39675b 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -309,10 +309,8 @@ static int is_atom(const char *atom, const char *s, int slen)
 }
 
 static void expand_atom(struct strbuf *sb, const char *atom, int len,
-			void *vdata)
+			struct expand_data *data)
 {
-	struct expand_data *data = vdata;
-
 	if (is_atom("objectname", atom, len)) {
 		if (!data->mark_query)
 			strbuf_addstr(sb, oid_to_hex(&data->oid));
@@ -346,19 +344,21 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len,
 		die("unknown format element: %.*s", len, atom);
 }
 
-static size_t expand_format(struct strbuf *sb, const char *start, void *data)
+static void expand_format(struct strbuf *sb, const char *start,
+			  struct expand_data *data)
 {
-	const char *end;
-
-	if (*start != '(')
-		return 0;
-	end = strchr(start + 1, ')');
-	if (!end)
-		die("format element '%s' does not end in ')'", start);
-
-	expand_atom(sb, start + 1, end - start - 1, data);
-
-	return end - start + 1;
+	while (strbuf_expand_step(sb, &start)) {
+		const char *end;
+
+		if (skip_prefix(start, "%", &start) || *start != '(')
+			strbuf_addch(sb, '%');
+		else if (!(end = strchr(start + 1, ')')))
+			die("format element '%s' does not end in ')'", start);
+		else {
+			expand_atom(sb, start + 1, end - start - 1, data);
+			start = end + 1;
+		}
+	}
 }
 
 static void batch_write(struct batch_options *opt, const void *data, int len)
@@ -494,7 +494,7 @@ static void batch_object_write(const char *obj_name,
 	if (!opt->format) {
 		print_default_format(scratch, data);
 	} else {
-		strbuf_expand(scratch, opt->format, expand_format, data);
+		expand_format(scratch, opt->format, data);
 		strbuf_addch(scratch, '\n');
 	}
 
@@ -777,9 +777,8 @@ static int batch_objects(struct batch_options *opt)
 	 */
 	memset(&data, 0, sizeof(data));
 	data.mark_query = 1;
-	strbuf_expand(&output,
+	expand_format(&output,
 		      opt->format ? opt->format : DEFAULT_FORMAT,
-		      expand_format,
 		      &data);
 	data.mark_query = 0;
 	strbuf_release(&output);
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 72012c0f0f..03bf5771b4 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -262,74 +262,57 @@ static void expand_objectsize(struct strbuf *line, const struct object_id *oid,
 		strbuf_addstr(line, "-");
 	}
 }
-struct show_index_data {
-	const char *pathname;
-	struct index_state *istate;
-	const struct cache_entry *ce;
-};
-
-static size_t expand_show_index(struct strbuf *sb, const char *start,
-				void *context)
-{
-	struct show_index_data *data = context;
-	const char *end;
-	const char *p;
-	size_t len = strbuf_expand_literal_cb(sb, start, NULL);
-	struct stat st;
-
-	if (len)
-		return len;
-	if (*start != '(')
-		die(_("bad ls-files format: element '%s' "
-		      "does not start with '('"), start);
-
-	end = strchr(start + 1, ')');
-	if (!end)
-		die(_("bad ls-files format: element '%s' "
-		      "does not end in ')'"), start);
-
-	len = end - start + 1;
-	if (skip_prefix(start, "(objectmode)", &p))
-		strbuf_addf(sb, "%06o", data->ce->ce_mode);
-	else if (skip_prefix(start, "(objectname)", &p))
-		strbuf_add_unique_abbrev(sb, &data->ce->oid, abbrev);
-	else if (skip_prefix(start, "(objecttype)", &p))
-		strbuf_addstr(sb, type_name(object_type(data->ce->ce_mode)));
-	else if (skip_prefix(start, "(objectsize:padded)", &p))
-		expand_objectsize(sb, &data->ce->oid, object_type(data->ce->ce_mode), 1);
-	else if (skip_prefix(start, "(objectsize)", &p))
-		expand_objectsize(sb, &data->ce->oid, object_type(data->ce->ce_mode), 0);
-	else if (skip_prefix(start, "(stage)", &p))
-		strbuf_addf(sb, "%d", ce_stage(data->ce));
-	else if (skip_prefix(start, "(eolinfo:index)", &p))
-		strbuf_addstr(sb, S_ISREG(data->ce->ce_mode) ?
-			      get_cached_convert_stats_ascii(data->istate,
-			      data->ce->name) : "");
-	else if (skip_prefix(start, "(eolinfo:worktree)", &p))
-		strbuf_addstr(sb, !lstat(data->pathname, &st) &&
-			      S_ISREG(st.st_mode) ?
-			      get_wt_convert_stats_ascii(data->pathname) : "");
-	else if (skip_prefix(start, "(eolattr)", &p))
-		strbuf_addstr(sb, get_convert_attr_ascii(data->istate,
-			      data->pathname));
-	else if (skip_prefix(start, "(path)", &p))
-		write_name_to_buf(sb, data->pathname);
-	else
-		die(_("bad ls-files format: %%%.*s"), (int)len, start);
-
-	return len;
-}
 
 static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,
 			const char *format, const char *fullname) {
-	struct show_index_data data = {
-		.pathname = fullname,
-		.istate = repo->index,
-		.ce = ce,
-	};
 	struct strbuf sb = STRBUF_INIT;
 
-	strbuf_expand(&sb, format, expand_show_index, &data);
+	while (strbuf_expand_step(&sb, &format)) {
+		const char *end;
+		size_t len;
+		struct stat st;
+
+		if (skip_prefix(format, "%", &format))
+			strbuf_addch(&sb, '%');
+		else if ((len = strbuf_expand_literal_cb(&sb, format, NULL)))
+			format += len;
+		else if (*format != '(')
+			die(_("bad ls-files format: element '%s' "
+			      "does not start with '('"), format);
+		else if (!(end = strchr(format + 1, ')')))
+			die(_("bad ls-files format: element '%s' "
+			      "does not end in ')'"), format);
+		else if (skip_prefix(format, "(objectmode)", &format))
+			strbuf_addf(&sb, "%06o", ce->ce_mode);
+		else if (skip_prefix(format, "(objectname)", &format))
+			strbuf_add_unique_abbrev(&sb, &ce->oid, abbrev);
+		else if (skip_prefix(format, "(objecttype)", &format))
+			strbuf_addstr(&sb, type_name(object_type(ce->ce_mode)));
+		else if (skip_prefix(format, "(objectsize:padded)", &format))
+			expand_objectsize(&sb, &ce->oid,
+					  object_type(ce->ce_mode), 1);
+		else if (skip_prefix(format, "(objectsize)", &format))
+			expand_objectsize(&sb, &ce->oid,
+					  object_type(ce->ce_mode), 0);
+		else if (skip_prefix(format, "(stage)", &format))
+			strbuf_addf(&sb, "%d", ce_stage(ce));
+		else if (skip_prefix(format, "(eolinfo:index)", &format))
+			strbuf_addstr(&sb, S_ISREG(ce->ce_mode) ?
+				      get_cached_convert_stats_ascii(repo->index,
+				      ce->name) : "");
+		else if (skip_prefix(format, "(eolinfo:worktree)", &format))
+			strbuf_addstr(&sb, !lstat(fullname, &st) &&
+				      S_ISREG(st.st_mode) ?
+				      get_wt_convert_stats_ascii(fullname) : "");
+		else if (skip_prefix(format, "(eolattr)", &format))
+			strbuf_addstr(&sb, get_convert_attr_ascii(repo->index,
+								 fullname));
+		else if (skip_prefix(format, "(path)", &format))
+			write_name_to_buf(&sb, fullname);
+		else
+			die(_("bad ls-files format: %%%.*s"),
+			    (int)(end - format + 1), format);
+	}
 	strbuf_addch(&sb, line_terminator);
 	fwrite(sb.buf, sb.len, 1, stdout);
 	strbuf_release(&sb);
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 077977a461..8460d20257 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -55,63 +55,6 @@ struct ls_tree_options {
 	const char *format;
 };
 
-struct show_tree_data {
-	struct ls_tree_options *options;
-	unsigned mode;
-	enum object_type type;
-	const struct object_id *oid;
-	const char *pathname;
-	struct strbuf *base;
-};
-
-static size_t expand_show_tree(struct strbuf *sb, const char *start,
-			       void *context)
-{
-	struct show_tree_data *data = context;
-	struct ls_tree_options *options = data->options;
-	const char *end;
-	const char *p;
-	unsigned int errlen;
-	size_t len = strbuf_expand_literal_cb(sb, start, NULL);
-
-	if (len)
-		return len;
-	if (*start != '(')
-		die(_("bad ls-tree format: element '%s' does not start with '('"), start);
-
-	end = strchr(start + 1, ')');
-	if (!end)
-		die(_("bad ls-tree format: element '%s' does not end in ')'"), start);
-
-	len = end - start + 1;
-	if (skip_prefix(start, "(objectmode)", &p)) {
-		strbuf_addf(sb, "%06o", data->mode);
-	} else if (skip_prefix(start, "(objecttype)", &p)) {
-		strbuf_addstr(sb, type_name(data->type));
-	} else if (skip_prefix(start, "(objectsize:padded)", &p)) {
-		expand_objectsize(sb, data->oid, data->type, 1);
-	} else if (skip_prefix(start, "(objectsize)", &p)) {
-		expand_objectsize(sb, data->oid, data->type, 0);
-	} else if (skip_prefix(start, "(objectname)", &p)) {
-		strbuf_add_unique_abbrev(sb, data->oid, options->abbrev);
-	} else if (skip_prefix(start, "(path)", &p)) {
-		const char *name = data->base->buf;
-		const char *prefix = options->chomp_prefix ? options->ls_tree_prefix : NULL;
-		struct strbuf sbuf = STRBUF_INIT;
-		size_t baselen = data->base->len;
-
-		strbuf_addstr(data->base, data->pathname);
-		name = relative_path(data->base->buf, prefix, &sbuf);
-		quote_c_style(name, sb, NULL, 0);
-		strbuf_setlen(data->base, baselen);
-		strbuf_release(&sbuf);
-	} else {
-		errlen = (unsigned long)len;
-		die(_("bad ls-tree format: %%%.*s"), errlen, start);
-	}
-	return len;
-}
-
 static int show_recursive(struct ls_tree_options *options, const char *base,
 			  size_t baselen, const char *pathname)
 {
@@ -150,14 +93,7 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,
 	int recurse = 0;
 	struct strbuf sb = STRBUF_INIT;
 	enum object_type type = object_type(mode);
-	struct show_tree_data cb_data = {
-		.options = options,
-		.mode = mode,
-		.type = type,
-		.oid = oid,
-		.pathname = pathname,
-		.base = base,
-	};
+	const char *format = options->format;
 
 	if (type == OBJ_TREE && show_recursive(options, base->buf, base->len, pathname))
 		recurse = READ_TREE_RECURSIVE;
@@ -166,7 +102,46 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,
 	if (type == OBJ_BLOB && (options->ls_options & LS_TREE_ONLY))
 		return 0;
 
-	strbuf_expand(&sb, options->format, expand_show_tree, &cb_data);
+	while (strbuf_expand_step(&sb, &format)) {
+		const char *end;
+		size_t len;
+
+		if (skip_prefix(format, "%", &format))
+			strbuf_addch(&sb, '%');
+		else if ((len = strbuf_expand_literal_cb(&sb, format, NULL)))
+			format += len;
+		else if (*format != '(')
+			die(_("bad ls-tree format: element '%s' "
+			      "does not start with '('"), format);
+		else if (!(end = strchr(format + 1, ')')))
+			die(_("bad ls-tree format: element '%s' "
+			      "does not end in ')'"), format);
+		else if (skip_prefix(format, "(objectmode)", &format))
+			strbuf_addf(&sb, "%06o", mode);
+		else if (skip_prefix(format, "(objecttype)", &format))
+			strbuf_addstr(&sb, type_name(type));
+		else if (skip_prefix(format, "(objectsize:padded)", &format))
+			expand_objectsize(&sb, oid, type, 1);
+		else if (skip_prefix(format, "(objectsize)", &format))
+			expand_objectsize(&sb, oid, type, 0);
+		else if (skip_prefix(format, "(objectname)", &format))
+			strbuf_add_unique_abbrev(&sb, oid, options->abbrev);
+		else if (skip_prefix(format, "(path)", &format)) {
+			const char *name;
+			const char *prefix = options->chomp_prefix ?
+					     options->ls_tree_prefix : NULL;
+			struct strbuf sbuf = STRBUF_INIT;
+			size_t baselen = base->len;
+
+			strbuf_addstr(base, pathname);
+			name = relative_path(base->buf, prefix, &sbuf);
+			quote_c_style(name, &sb, NULL, 0);
+			strbuf_setlen(base, baselen);
+			strbuf_release(&sbuf);
+		} else
+			die(_("bad ls-tree format: %%%.*s"),
+			    (int)(end - format + 1), format);
+	}
 	strbuf_addch(&sb, options->null_termination ? '\0' : '\n');
 	fwrite(sb.buf, sb.len, 1, stdout);
 	strbuf_release(&sb);
diff --git a/daemon.c b/daemon.c
index 7139cc201d..3682bfdd08 100644
--- a/daemon.c
+++ b/daemon.c
@@ -144,42 +144,6 @@ static void NORETURN daemon_die(const char *err, va_list params)
 	exit(1);
 }
 
-struct expand_path_context {
-	const char *directory;
-	struct hostinfo *hostinfo;
-};
-
-static size_t expand_path(struct strbuf *sb, const char *placeholder, void *ctx)
-{
-	struct expand_path_context *context = ctx;
-	struct hostinfo *hi = context->hostinfo;
-
-	switch (placeholder[0]) {
-	case 'H':
-		strbuf_addbuf(sb, &hi->hostname);
-		return 1;
-	case 'C':
-		if (placeholder[1] == 'H') {
-			strbuf_addstr(sb, get_canon_hostname(hi));
-			return 2;
-		}
-		break;
-	case 'I':
-		if (placeholder[1] == 'P') {
-			strbuf_addstr(sb, get_ip_address(hi));
-			return 2;
-		}
-		break;
-	case 'P':
-		strbuf_addbuf(sb, &hi->tcp_port);
-		return 1;
-	case 'D':
-		strbuf_addstr(sb, context->directory);
-		return 1;
-	}
-	return 0;
-}
-
 static const char *path_ok(const char *directory, struct hostinfo *hi)
 {
 	static char rpath[PATH_MAX];
@@ -223,10 +187,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)
 	}
 	else if (interpolated_path && hi->saw_extended_args) {
 		struct strbuf expanded_path = STRBUF_INIT;
-		struct expand_path_context context;
-
-		context.directory = directory;
-		context.hostinfo = hi;
+		const char *format = interpolated_path;
 
 		if (*dir != '/') {
 			/* Allow only absolute */
@@ -234,8 +195,24 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)
 			return NULL;
 		}
 
-		strbuf_expand(&expanded_path, interpolated_path,
-			      expand_path, &context);
+		while (strbuf_expand_step(&expanded_path, &format)) {
+			if (skip_prefix(format, "%", &format))
+				strbuf_addch(&expanded_path, '%');
+			else if (skip_prefix(format, "H", &format))
+				strbuf_addbuf(&expanded_path, &hi->hostname);
+			else if (skip_prefix(format, "CH", &format))
+				strbuf_addstr(&expanded_path,
+					      get_canon_hostname(hi));
+			else if (skip_prefix(format, "IP", &format))
+				strbuf_addstr(&expanded_path,
+					      get_ip_address(hi));
+			else if (skip_prefix(format, "P", &format))
+				strbuf_addbuf(&expanded_path, &hi->tcp_port);
+			else if (skip_prefix(format, "D", &format))
+				strbuf_addstr(&expanded_path, directory);
+			else
+				strbuf_addch(&expanded_path, '%');
+		}
 
 		rlen = strlcpy(interp_path, expanded_path.buf,
 			       sizeof(interp_path));
diff --git a/pretty.c b/pretty.c
index d2df561a05..cffbf32987 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1254,9 +1254,19 @@ static struct strbuf *expand_separator(struct strbuf *sb,
 				       const char *argval, size_t arglen)
 {
 	char *fmt = xstrndup(argval, arglen);
+	const char *format = fmt;
 
 	strbuf_reset(sb);
-	strbuf_expand(sb, fmt, strbuf_expand_literal_cb, NULL);
+	while (strbuf_expand_step(sb, &format)) {
+		size_t len;
+
+		if (skip_prefix(format, "%", &format))
+			strbuf_addch(sb, '%');
+		else if ((len = strbuf_expand_literal_cb(sb, format, NULL)))
+			format += len;
+		else
+			strbuf_addch(sb, '%');
+	}
 	free(fmt);
 	return sb;
 }
@@ -1803,7 +1813,7 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
 
 static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */
 				 const char *placeholder,
-				 void *context)
+				 struct format_commit_context *context)
 {
 	size_t consumed, orig_len;
 	enum {
@@ -1842,7 +1852,7 @@ static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */
 	}
 
 	orig_len = sb->len;
-	if (((struct format_commit_context *)context)->flush_type != no_flush)
+	if ((context)->flush_type != no_flush)
 		consumed = format_and_pad_commit(sb, placeholder, context);
 	else
 		consumed = format_commit_one(sb, placeholder, context);
@@ -1861,30 +1871,6 @@ static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */
 	return consumed + 1;
 }
 
-static size_t userformat_want_item(struct strbuf *sb UNUSED,
-				   const char *placeholder,
-				   void *context)
-{
-	struct userformat_want *w = context;
-
-	if (*placeholder == '+' || *placeholder == '-' || *placeholder == ' ')
-		placeholder++;
-
-	switch (*placeholder) {
-	case 'N':
-		w->notes = 1;
-		break;
-	case 'S':
-		w->source = 1;
-		break;
-	case 'd':
-	case 'D':
-		w->decorate = 1;
-		break;
-	}
-	return 0;
-}
-
 void userformat_find_requirements(const char *fmt, struct userformat_want *w)
 {
 	struct strbuf dummy = STRBUF_INIT;
@@ -1894,7 +1880,26 @@ void userformat_find_requirements(const char *fmt, struct userformat_want *w)
 			return;
 		fmt = user_format;
 	}
-	strbuf_expand(&dummy, fmt, userformat_want_item, w);
+	while (strbuf_expand_step(&dummy, &fmt)) {
+		if (skip_prefix(fmt, "%", &fmt))
+			continue;
+
+		if (*fmt == '+' || *fmt == '-' || *fmt == ' ')
+			fmt++;
+
+		switch (*fmt) {
+		case 'N':
+			w->notes = 1;
+			break;
+		case 'S':
+			w->source = 1;
+			break;
+		case 'd':
+		case 'D':
+			w->decorate = 1;
+			break;
+		}
+	}
 	strbuf_release(&dummy);
 }
 
@@ -1912,7 +1917,16 @@ void repo_format_commit_message(struct repository *r,
 	const char *output_enc = pretty_ctx->output_encoding;
 	const char *utf8 = "UTF-8";
 
-	strbuf_expand(sb, format, format_commit_item, &context);
+	while (strbuf_expand_step(sb, &format)) {
+		size_t len;
+
+		if (skip_prefix(format, "%", &format))
+			strbuf_addch(sb, '%');
+		else if ((len = format_commit_item(sb, format, &context)))
+			format += len;
+		else
+			strbuf_addch(sb, '%');
+	}
 	rewrap_message_tail(sb, &context, 0, 0, 0);
 
 	/*
diff --git a/strbuf.c b/strbuf.c
index 972366b410..c3d1cee616 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -427,26 +427,6 @@ int strbuf_expand_step(struct strbuf *sb, const char **formatp)
 	return 1;
 }
 
-void strbuf_expand(struct strbuf *sb, const char *format, expand_fn_t fn,
-		   void *context)
-{
-	while (strbuf_expand_step(sb, &format)) {
-		size_t consumed;
-
-		if (*format == '%') {
-			strbuf_addch(sb, '%');
-			format++;
-			continue;
-		}
-
-		consumed = fn(sb, format, context);
-		if (consumed)
-			format += consumed;
-		else
-			strbuf_addch(sb, '%');
-	}
-}
-
 size_t strbuf_expand_literal_cb(struct strbuf *sb,
 				const char *placeholder,
 				void *context UNUSED)
diff --git a/strbuf.h b/strbuf.h
index e293117f07..95e50e243e 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -318,40 +318,9 @@ const char *strbuf_join_argv(struct strbuf *buf, int argc,
 			     const char **argv, char delim);
 
 /**
- * This function can be used to expand a format string containing
- * placeholders. To that end, it parses the string and calls the specified
- * function for every percent sign found.
- *
- * The callback function is given a pointer to the character after the `%`
- * and a pointer to the struct strbuf.  It is expected to add the expanded
- * version of the placeholder to the strbuf, e.g. to add a newline
- * character if the letter `n` appears after a `%`.  The function returns
- * the length of the placeholder recognized and `strbuf_expand()` skips
- * over it.
- *
- * The format `%%` is automatically expanded to a single `%` as a quoting
- * mechanism; callers do not need to handle the `%` placeholder themselves,
- * and the callback function will not be invoked for this placeholder.
- *
- * All other characters (non-percent and not skipped ones) are copied
- * verbatim to the strbuf.  If the callback returned zero, meaning that the
- * placeholder is unknown, then the percent sign is copied, too.
- *
- * In order to facilitate caching and to make it possible to give
- * parameters to the callback, `strbuf_expand()` passes a context
- * pointer with any kind of data.
- */
-typedef size_t (*expand_fn_t) (struct strbuf *sb,
-			       const char *placeholder,
-			       void *context);
-void strbuf_expand(struct strbuf *sb,
-		   const char *format,
-		   expand_fn_t fn,
-		   void *context);
-
-/**
- * Used as callback for `strbuf_expand` to only expand literals
- * (i.e. %n and %xNN). The context argument is ignored.
+ * Used with `strbuf_expand_step` to expand the literals %n and %x
+ * followed by two hexadecimal digits. Returns the number of recognized
+ * characters. The context argument is ignored.
  */
 size_t strbuf_expand_literal_cb(struct strbuf *sb,
 				const char *placeholder,