"Hariom Verma via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > -static void format_sanitized_subject(struct strbuf *sb, const char *msg) > +static void format_sanitized_subject(struct strbuf *sb, const char *msg, size_t len) > { > + char *r = xmemdupz(msg, len); > size_t trimlen; > size_t start_len = sb->len; > int space = 2; > + int i; > > - for (; *msg && *msg != '\n'; msg++) { > - if (istitlechar(*msg)) { > + for (i = 0; i < len; i++) { > + if (r[i] == '\n') > + r[i] = ' '; Copying the whole string only for this one looks very wasteful. Can't you do for (i = 0; i < len; i++) { char r = msg[i]; if (isspace(r)) r = ' '; if (istitlechar(r)) { ... } or something like that instead? > + if (istitlechar(r[i])) { > if (space == 1) > strbuf_addch(sb, '-'); > space = 0; > - strbuf_addch(sb, *msg); > - if (*msg == '.') > - while (*(msg+1) == '.') > - msg++; > + strbuf_addch(sb, r[i]); > + if (r[i] == '.') > + while (r[i+1] == '.') > + i++; > } else > space |= 1; > } > + free(r); Also, because neither LF or SP is a titlechar(), wouldn't the "if r[i] is LF, replace it with SP" a no-op wrt what will be in sb at the end? > case 'f': /* sanitized subject */ > - format_sanitized_subject(sb, msg + c->subject_off); > + eol = strchrnul(msg + c->subject_off, '\n'); > + format_sanitized_subject(sb, msg + c->subject_off, eol - (msg + c->subject_off)); This original caller expected the helper to stop reading at the end of the first line, but the updated helper needs to be told where to stop, so we do so with some extra computation. Makes sense.