Re: [PATCH 2/4] trailer: avoid unnecessary splitting on lines

From: Christian Couder <christian.couder@gmail.com>
Date: Oct 29, 2016, 12:25 UTC
Message-ID: <CAP8UFD3o89EzQLLsMtLWxBhKCG4RmbG6u+UrLwzyAAEyezv1cA@mail.gmail.com>
In-Reply-To: <db609e13740415ac4e5e357493661347b0f681f7.1477698917.git.jonathantanmy@google.com>

On Sat, Oct 29, 2016 at 2:05 AM, Jonathan Tan <jonathantanmy@google.com> wrote:

Show 15 quoted lines

> trailer.c currently splits lines while processing a buffer (and also
> rejoins lines when needing to invoke ignore_non_trailer).
>
> Avoid such line splitting, except when generating the strings
> corresponding to trailers (for ease of use by clients - a subsequent
> patch will allow other components to obtain the layout of a trailer
> block in a buffer, including the trailers themselves). The main purpose
> of this is to make it easy to return pointers into the original buffer
> (for a subsequent patch), but this also significantly reduces the number
> of memory allocations required.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  trailer.c | 215 +++++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 116 insertions(+), 99 deletions(-)

IMHO it is telling that this needs 17 more lines.

Show 6 quoted lines

> @@ -954,7 +971,7 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str
>  {
>         LIST_HEAD(head);
>         LIST_HEAD(arg_head);
> -       struct strbuf **lines;
> +       struct strbuf sb = STRBUF_INIT;

We often use "sb" as the name of strbuf variables, but I think at least here (and maybe in other places above) we could use something a bit more telling, like "input_buf" perhaps.

>         int trailer_end;
>         FILE *outfile = stdout;
>