Jonathan Tan <jonathanta...@google.com> 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.
