Skip to content

Commit f930a23

Browse files
pks-tgitster
authored andcommitted
utf8: refactor strbuf_utf8_replace to not rely on preallocated buffer
In `strbuf_utf8_replace`, we preallocate the destination buffer and then use `memcpy` to copy bytes into it at computed offsets. This feels rather fragile and is hard to understand at times. Refactor the code to instead use `strbuf_add` and `strbuf_addstr` so that we can be sure that there is no possibility to perform an out-of-bounds write. Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 81c2d4c commit f930a23

File tree

1 file changed

+13
-21
lines changed

1 file changed

+13
-21
lines changed

utf8.c

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -365,26 +365,20 @@ void strbuf_add_wrapped_bytes(struct strbuf *buf, const char *data, int len,
365365
void strbuf_utf8_replace(struct strbuf *sb_src, int pos, int width,
366366
const char *subst)
367367
{
368-
struct strbuf sb_dst = STRBUF_INIT;
369-
char *src = sb_src->buf;
370-
char *end = src + sb_src->len;
371-
char *dst;
372-
int w = 0, subst_len = 0;
368+
const char *src = sb_src->buf, *end = sb_src->buf + sb_src->len;
369+
struct strbuf dst;
370+
int w = 0;
373371

374-
if (subst)
375-
subst_len = strlen(subst);
376-
strbuf_grow(&sb_dst, sb_src->len + subst_len);
377-
dst = sb_dst.buf;
372+
strbuf_init(&dst, sb_src->len);
378373

379374
while (src < end) {
375+
const char *old;
380376
int glyph_width;
381-
char *old;
382377
size_t n;
383378

384379
while ((n = display_mode_esc_sequence_len(src))) {
385-
memcpy(dst, src, n);
380+
strbuf_add(&dst, src, n);
386381
src += n;
387-
dst += n;
388382
}
389383

390384
if (src >= end)
@@ -404,21 +398,19 @@ void strbuf_utf8_replace(struct strbuf *sb_src, int pos, int width,
404398

405399
if (glyph_width && w >= pos && w < pos + width) {
406400
if (subst) {
407-
memcpy(dst, subst, subst_len);
408-
dst += subst_len;
401+
strbuf_addstr(&dst, subst);
409402
subst = NULL;
410403
}
411-
w += glyph_width;
412-
continue;
404+
} else {
405+
strbuf_add(&dst, old, src - old);
413406
}
414-
memcpy(dst, old, src - old);
415-
dst += src - old;
407+
416408
w += glyph_width;
417409
}
418-
strbuf_setlen(&sb_dst, dst - sb_dst.buf);
419-
strbuf_swap(sb_src, &sb_dst);
410+
411+
strbuf_swap(sb_src, &dst);
420412
out:
421-
strbuf_release(&sb_dst);
413+
strbuf_release(&dst);
422414
}
423415

424416
/*

0 commit comments

Comments
 (0)