replace strbuf_expand_dict_cb() with strbuf_expand_step()

Avoid the overhead of setting up a dictionary and passing it via strbuf_expand() to strbuf_expand_dict_cb() by using strbuf_expand_step() in a loop instead. It requires explicit handling of %% and unrecognized placeholders, but is more direct and simpler overall, and expands only on demand. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

René Scharfe committed Jun 17, 2023 at 22:42 UTC 39dbd49b4138b6cdc9fb73e317d4e9f06df0c5c5
4 files changed +28 -56
convert.c
+10 -12
@@ -633,23 +633,21 @@ static int filter_buffer_or_fd(int in UNUSED, int out, void *data)
633 */
634 struct child_process child_process = CHILD_PROCESS_INIT;
635 struct filter_params *params = (struct filter_params *)data;
636 + const char *format = params->cmd;
637 int write_err, status;
638
639 /* apply % substitution to cmd */
640 struct strbuf cmd = STRBUF_INIT;
640 - struct strbuf path = STRBUF_INIT;
641 - struct strbuf_expand_dict_entry dict[] = {
642 - { "f", NULL, },
643 - { NULL, NULL, },
644 - };
645 -
646 - /* quote the path to preserve spaces, etc. */
647 - sq_quote_buf(&path, params->path);
648 - dict[0].value = path.buf;
641
650 - /* expand all %f with the quoted path */
651 - strbuf_expand(&cmd, params->cmd, strbuf_expand_dict_cb, &dict);
652 - strbuf_release(&path);
642 + /* expand all %f with the quoted path; quote to preserve space, etc. */
643 + while (strbuf_expand_step(&cmd, &format)) {
644 + if (skip_prefix(format, "%", &format))
645 + strbuf_addch(&cmd, '%');
646 + else if (skip_prefix(format, "f", &format))
647 + sq_quote_buf(&cmd, params->path);
648 + else
649 + strbuf_addch(&cmd, '%');
650 + }
651
652 strvec_push(&child_process.args, cmd.buf);
653 child_process.use_shell = 1;
ll-merge.c
+18 -14
@@ -192,24 +192,15 @@ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
192 const struct ll_merge_options *opts,
193 int marker_size)
194 {
195 - char temp[4][50];
195 + char temp[3][50];
196 struct strbuf cmd = STRBUF_INIT;
197 - struct strbuf_expand_dict_entry dict[6];
198 - struct strbuf path_sq = STRBUF_INIT;
197 + const char *format = fn->cmdline;
198 struct child_process child = CHILD_PROCESS_INIT;
199 int status, fd, i;
200 struct stat st;
201 enum ll_merge_result ret;
202 assert(opts);
203
205 - sq_quote_buf(&path_sq, path);
206 - dict[0].placeholder = "O"; dict[0].value = temp[0];
207 - dict[1].placeholder = "A"; dict[1].value = temp[1];
208 - dict[2].placeholder = "B"; dict[2].value = temp[2];
209 - dict[3].placeholder = "L"; dict[3].value = temp[3];
210 - dict[4].placeholder = "P"; dict[4].value = path_sq.buf;
211 - dict[5].placeholder = NULL; dict[5].value = NULL;
212 -
204 if (!fn->cmdline)
205 die("custom merge driver %s lacks command line.", fn->name);
206
@@ -218,9 +209,23 @@ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
209 create_temp(orig, temp[0], sizeof(temp[0]));
210 create_temp(src1, temp[1], sizeof(temp[1]));
211 create_temp(src2, temp[2], sizeof(temp[2]));
221 - xsnprintf(temp[3], sizeof(temp[3]), "%d", marker_size);
212
223 - strbuf_expand(&cmd, fn->cmdline, strbuf_expand_dict_cb, &dict);
213 + while (strbuf_expand_step(&cmd, &format)) {
214 + if (skip_prefix(format, "%", &format))
215 + strbuf_addch(&cmd, '%');
216 + else if (skip_prefix(format, "O", &format))
217 + strbuf_addstr(&cmd, temp[0]);
218 + else if (skip_prefix(format, "A", &format))
219 + strbuf_addstr(&cmd, temp[1]);
220 + else if (skip_prefix(format, "B", &format))
221 + strbuf_addstr(&cmd, temp[2]);
222 + else if (skip_prefix(format, "L", &format))
223 + strbuf_addf(&cmd, "%d", marker_size);
224 + else if (skip_prefix(format, "P", &format))
225 + sq_quote_buf(&cmd, path);
226 + else
227 + strbuf_addch(&cmd, '%');
228 + }
229
230 child.use_shell = 1;
231 strvec_push(&child.args, cmd.buf);
@@ -242,7 +247,6 @@ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
247 for (i = 0; i < 3; i++)
248 unlink_or_warn(temp[i]);
249 strbuf_release(&cmd);
245 - strbuf_release(&path_sq);
250 ret = (status > 0) ? LL_MERGE_CONFLICT : status;
251 return ret;
252 }
strbuf.c
-16
@@ -468,22 +468,6 @@ size_t strbuf_expand_literal_cb(struct strbuf *sb,
468 return 0;
469 }
470
471 -size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder,
472 - void *context)
473 -{
474 - struct strbuf_expand_dict_entry *e = context;
475 - size_t len;
476 -
477 - for (; e->placeholder && (len = strlen(e->placeholder)); e++) {
478 - if (!strncmp(placeholder, e->placeholder, len)) {
479 - if (e->value)
480 - strbuf_addstr(sb, e->value);
481 - return len;
482 - }
483 - }
484 - return 0;
485 -}
486 -
471 void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src)
472 {
473 size_t i, len = src->len;
strbuf.h
-14
@@ -357,20 +357,6 @@ size_t strbuf_expand_literal_cb(struct strbuf *sb,
357 const char *placeholder,
358 void *context);
359
360 -/**
361 - * Used as callback for `strbuf_expand()`, expects an array of
362 - * struct strbuf_expand_dict_entry as context, i.e. pairs of
363 - * placeholder and replacement string. The array needs to be
364 - * terminated by an entry with placeholder set to NULL.
365 - */
366 -struct strbuf_expand_dict_entry {
367 - const char *placeholder;
368 - const char *value;
369 -};
370 -size_t strbuf_expand_dict_cb(struct strbuf *sb,
371 - const char *placeholder,
372 - void *context);
373 -
360 /**
361 * If the string pointed to by `formatp` contains a percent sign ("%"),
362 * advance it to point to the character following the next one and