Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes: > static char *separators = ":"; > > +static int configured = 0; Avoid initializing a static to 0 or NULL; instead let .bss take care of it. > static const char *token_from_item(struct arg_item *item, char *tok) > { > if (item->conf.key) > @@ -872,59 +885,43 @@ static int process_input_file(FILE *outfile, > const char *str, > struct list_head *head) > { > - int patch_start, trailer_start, trailer_end; > + struct trailer_info info; > struct strbuf tok = STRBUF_INIT; > struct strbuf val = STRBUF_INIT; > - struct trailer_item *last = NULL; > - struct strbuf *trailer, **trailer_lines, **ptr; > + int i; > > - patch_start = find_patch_start(str); > - trailer_end = find_trailer_end(str, patch_start); > - trailer_start = find_trailer_start(str, trailer_end); > + trailer_info_get(&info, str); OK, it needs a reading of trailer_info_get() first to understand the remainder of this function. The function would grab these fields into info, among doing other things. > /* Print lines before the trailers as is */ > - fwrite(str, 1, trailer_start, outfile); > + fwrite(str, 1, info.trailer_start - str, outfile); > > - if (!ends_with_blank_line(str, trailer_start)) > + if (!info.blank_line_before_trailer) > fprintf(outfile, "\n"); ... and one of the "other things" include setting the ->blank_line_before_trailer field. > - /* Parse trailer lines */ > - trailer_lines = strbuf_split_buf(str + trailer_start, > - trailer_end - trailer_start, > - '\n', > - 0); > - for (ptr = trailer_lines; *ptr; ptr++) { > + for (i = 0; i < info.trailer_nr; i++) { > int separator_pos; > - trailer = *ptr; > - if (trailer->buf[0] == comment_line_char) > + char *trailer = info.trailers[i]; > + if (trailer[0] == comment_line_char) > continue; And info.trailers[] is no longer an array of strbuf; it is an array of "char *", which I alluded to during my review of [2/4]. Looking good. > - if (last && isspace(trailer->buf[0])) { > - struct strbuf sb = STRBUF_INIT; > - strbuf_addf(&sb, "%s\n%s", last->value, trailer->buf); > - strbuf_strip_suffix(&sb, "\n"); > - free(last->value); > - last->value = strbuf_detach(&sb, NULL); > - continue; > - } > - separator_pos = find_separator(trailer->buf, separators); > + separator_pos = find_separator(trailer, separators); ... presumably, the line-folding is already handled in trailer_info_get(), so we do not need the "last" handling. > if (separator_pos >= 1) { ... and it it is a "mail-header: looking" one, then add it one way. > - parse_trailer(&tok, &val, NULL, trailer->buf, > - separator_pos); > - last = add_trailer_item(head, > - strbuf_detach(&tok, NULL), > - strbuf_detach(&val, NULL)); > + parse_trailer(&tok, &val, NULL, trailer, > + separator_pos); > + add_trailer_item(head, > + strbuf_detach(&tok, NULL), > + strbuf_detach(&val, NULL)); > } else { > - strbuf_addbuf(&val, trailer); > + strbuf_addstr(&val, trailer); ... otherwise add it another way. > strbuf_strip_suffix(&val, "\n"); > add_trailer_item(head, > NULL, > strbuf_detach(&val, NULL)); > - last = NULL; > } > } > - strbuf_list_free(trailer_lines); > > - return trailer_end; > + trailer_info_release(&info); > + > + return info.trailer_end - str; > } Nicely done. > @@ -1004,3 +999,54 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str > > strbuf_release(&sb); > } > + > +void trailer_info_get(struct trailer_info *info, const char *str) > +{ > + int patch_start, trailer_end, trailer_start; > + struct strbuf **trailer_lines, **ptr; > + char **trailer_strings = NULL; > + size_t nr = 0, alloc = 0; > + char **last = NULL; > + > + ensure_configured(); > + > + patch_start = find_patch_start(str); > + trailer_end = find_trailer_end(str, patch_start); > + trailer_start = find_trailer_start(str, trailer_end); > + > + trailer_lines = strbuf_split_buf(str + trailer_start, > + trailer_end - trailer_start, > + '\n', > + 0); > + for (ptr = trailer_lines; *ptr; ptr++) { > + if (last && isspace((*ptr)->buf[0])) { > + struct strbuf sb = STRBUF_INIT; > + strbuf_attach(&sb, *last, strlen(*last), strlen(*last)); > + strbuf_addbuf(&sb, *ptr); > + *last = strbuf_detach(&sb, NULL); > + continue; > + } > + ALLOC_GROW(trailer_strings, nr + 1, alloc); > + trailer_strings[nr] = strbuf_detach(*ptr, NULL); > + last = find_separator(trailer_strings[nr], separators) >= 1 > + ? &trailer_strings[nr] > + : NULL; > + nr++; > + } > + strbuf_list_free(trailer_lines); > + > + info->blank_line_before_trailer = ends_with_blank_line(str, > + trailer_start); > + info->trailer_start = str + trailer_start; > + info->trailer_end = str + trailer_end; > + info->trailers = trailer_strings; > + info->trailer_nr = nr; > +} OK, looking good.