In-tree strbuf "in-place" search/replace (was: [PATCH v2 6/8] merge-ort: format messages slightly different for use in headers)
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-12-26 18:47:29
On Sat, Dec 25 2021, Elijah Newren via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
@@ -634,17 +634,46 @@ static void path_msg(struct merge_options *opt, const char *fmt, ...) { va_list ap; - struct strbuf *sb = strmap_get(&opt->priv->output, path); + struct strbuf *sb, *dest; + struct strbuf tmp = STRBUF_INIT; + + if (opt->record_conflict_msgs_as_headers && omittable_hint) + return; /* Do not record mere hints in tree */ + sb = strmap_get(&opt->priv->output, path); if (!sb) { sb = xmalloc(sizeof(*sb)); strbuf_init(sb, 0); strmap_put(&opt->priv->output, path, sb); } + dest = (opt->record_conflict_msgs_as_headers ? &tmp : sb); + va_start(ap, fmt); - strbuf_vaddf(sb, fmt, ap); + strbuf_vaddf(dest, fmt, ap); va_end(ap); + if (opt->record_conflict_msgs_as_headers) { + int i_sb = 0, i_tmp = 0; + + /* Copy tmp to sb, adding spaces after newlines */ + strbuf_grow(sb, 2*tmp.len); /* more than sufficient */ + for (; i_tmp < tmp.len; i_tmp++, i_sb++) { + /* Copy next character from tmp to sb */ + sb->buf[sb->len + i_sb] = tmp.buf[i_tmp]; + + /* If we copied a newline, add a space */ + if (tmp.buf[i_tmp] == '\n') + sb->buf[++i_sb] = ' '; + } + /* Update length and ensure it's NUL-terminated */ + sb->len += i_sb; + sb->buf[sb->len] = '\0'; + + /* Clean up tmp */ + strbuf_release(&tmp); + } + + /* Add final newline character to sb */ strbuf_addch(sb, '\n'); }
I'm not saying this is wrong or needs to change. Just a reader's note
that this sent me on an interesting journey of looking at various
in-tree callers of strbufs that want to do the equivalent of
s/$from/$to/ on a strbuf, with and without the equivalent of /g.
I figured I'd change the $subject since this is more of a general
musing...
In trailer.c we've got strbuf_replace(), which looks like it could be
made to be general enough to serve most callers if it did a memmem()
instead of a strstr(), and knew to take a "all" flag to implement a /g.
We then have e.g. lf_to_crlf() in imap-send.c, which uses a newly
alloc'd buffer followed by a strbuf_attach(), which is a common pattern.
Then strbuf_reencode() in strbuf.c basically solves this problem, and
calls reencode_string_len(), both it and the underlying function are
*almost* general enough to know to take some "from/to" string/length
pair, i.e. to not be bound to "reencoding" only with iconv().
Then there's strbuf_add_percentencode() and strbuf_add_urlencode() whose
API users might be happy with in-place replacing, but do a
read-and-copy-maybe-expand.
It might be an interesting follow-up project for someone to come up with
a generic in-place search-replace function with a signature like:
int strbuf_replace(struct strbuf *sb, const char *from,
size_t from_len, const char *to,
size_t to_len, int max);
To do e.g. in this case:
if (opt->record_conflict_msgs_as_headers)
strbuf_replace(sb, "\n", strlen("\n"), "\n ", strlen("\n "), -1);
The various in-tree implementations do some variant of over-mallocing to
save work in the loop (as is being done here), copying where a small
realloc/memmove might do, scanning the string to figure out how much to
malloc, then copying in a second pass etc.