trailer: fix leaking strbufs when formatting trailers

When formatting trailer lines we iterate through each of the trailers
and munge their respective token/value pairs according to the trailer
options. When formatting a trailer that has its `item->token` pointer
set we perform the munging in two local buffers. In the case where we
figure out that the value is empty and `trim_empty` is set we just skip
over the trailer item. But the buffers are local to the loop and we
don't release their contents, leading to a memory leak.

Plug this leak by lifting the buffers outside of the loop and releasing
them on function return. This fixes the memory leaks, but also optimizes
the loop as we don't have to reallocate the buffers on every single
iteration.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 0f7d8938d984d9a2c560b4b8d491c10124a9c301..38d6ccaa001f908535ec1c4e7e333dbfb8b59f88 100755 (executable)
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -5,6 +5,7 @@
 
 test_description='git interpret-trailers'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 # When we want one trailing space at the end of each line, let's use sed

diff --git a/trailer.c b/trailer.c
index 6bafe92b3266d58a831bbe47cafc55ff0cf980c7..8ba350404d4692b26c50d743e91e042a8af44c09 100644 (file)
--- a/trailer.c
+++ b/trailer.c
@@ -1111,6 +1111,8 @@ void format_trailers(const struct process_trailer_options *opts,
                     struct list_head *trailers,
                     struct strbuf *out)
 {
+       struct strbuf tok = STRBUF_INIT;
+       struct strbuf val = STRBUF_INIT;
        size_t origlen = out->len;
        struct list_head *pos;
        struct trailer_item *item;
@@ -1118,9 +1120,9 @@ void format_trailers(const struct process_trailer_options *opts,
        list_for_each(pos, trailers) {
                item = list_entry(pos, struct trailer_item, list);
                if (item->token) {
-                       struct strbuf tok = STRBUF_INIT;
-                       struct strbuf val = STRBUF_INIT;
+                       strbuf_reset(&tok);
                        strbuf_addstr(&tok, item->token);
+                       strbuf_reset(&val);
                        strbuf_addstr(&val, item->value);
 
                        /*
@@ -1151,9 +1153,6 @@ void format_trailers(const struct process_trailer_options *opts,
                                if (!opts->separator)
                                        strbuf_addch(out, '\n');
                        }
-                       strbuf_release(&tok);
-                       strbuf_release(&val);
-
                } else if (!opts->only_trailers) {
                        if (opts->separator && out->len != origlen) {
                                strbuf_addbuf(out, opts->separator);
@@ -1165,6 +1164,9 @@ void format_trailers(const struct process_trailer_options *opts,
                                strbuf_addch(out, '\n');
                }
        }
+
+       strbuf_release(&tok);
+       strbuf_release(&val);
 }
 
 void format_trailers_from_commit(const struct process_trailer_options *opts,