Re: [PATCH v3 7/9] pretty: refactor `format_sanitized_subject()`
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 17, 2020, 19:29 UTC
- Message-ID
- <xmqqpn7p1373.fsf@gitster.c.googlers.com>
- In-Reply-To
- <0ad22c7cdd3c692aa5b46444e64a3b76f1e87b4c.1597687822.git.gitgitgadget@gmail.com>
"Hariom Verma via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 14 quoted lines
> -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?
Show 16 quoted lines
> + 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.