Subject: Re: [PATCH 10/21] trailer: fix leaking trailer values

From: Toon Claes <[email-masked]>
Date: Fri, 18 Oct 2024 14:03:26 +0200

Patrick Steinhardt <[email-masked]> writes:

> Fix leaking trailer values when replacing the value with a command or
> when the token value is empty.
>
> Signed-off-by: Patrick Steinhardt <[email-masked]>
> ---
>
> diff --git a/trailer.c b/trailer.c
> index 682d74505bf..5c0bfb735a9 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -1124,7 +1131,7 @@ void format_trailers(const struct process_trailer_options *opts,
>  			 * corresponding value).
>  			 */
>  			if (opts->trim_empty && !strlen(item->value))
> -				continue;
> +				goto next;

While this is technically correct, I found it rather hard to understand
what's happening. What do you think about instead turning the `if` below
in an `else if` ?

>  
>  			if (!opts->filter || opts->filter(&tok, opts->filter_data)) {
>  				if (opts->separator && out->len != origlen)
> @@ -1145,9 +1152,10 @@ void format_trailers(const struct process_trailer_options *opts,
>  				if (!opts->separator)
>  					strbuf_addch(out, '\n');
>  			}
> +
> +next:
>  			strbuf_release(&tok);
>  			strbuf_release(&val);
> -
>  		} else if (!opts->only_trailers) {
>  			if (opts->separator && out->len != origlen) {
>  				strbuf_addbuf(out, opts->separator);
> -- 
> 2.47.0.dirty

--
Toon